Skip to content

Output Immersed Boundary load and state data#1302

Merged
sbryngelson merged 17 commits intoMFlowCode:masterfrom
mrvandenboom:ibdata-output
Mar 16, 2026
Merged

Output Immersed Boundary load and state data#1302
sbryngelson merged 17 commits intoMFlowCode:masterfrom
mrvandenboom:ibdata-output

Conversation

@mrvandenboom
Copy link
Contributor

Description

This adds binary output of IB load and state data during the simulation run, and conversion to a CSV file in post processing. Including the parameter ib_state_wrt="T" will activate the output routines. Output files can be found under the "/D" folder.

Fixes #(issue)

Type of change

  • Bug fix
  • [ X] New feature
  • Refactor
  • Documentation
  • Other: describe

Testing

Ran lint, format, test -a on a MacBookPro (10 core) and on (4) Tuo GPUs.

Checklist

  • I added or updated tests for new behavior
  • [X ] I updated documentation if user-facing behavior changed

See the developer guide for full coding standards.

GPU changes (expand if you modified src/simulation/)
  • GPU results match CPU results
  • [ X] Tested on NVIDIA GPU or AMD GPU

Tests ran considerably faster on MacBook CPUs than on Tuolumne GPUs.

AI code reviews

Reviews are not triggered automatically. To request a review, comment on the PR:

  • @coderabbitai review — incremental review (new changes only)
  • /review — Qodo review
  • /improve — Qodo code suggestions

mrvandenboom and others added 15 commits February 26, 2026 21:25
organize the code.  Adds new case parameter ib_force_wrt to toggle output.
This reverts commit 647d4c0.
By reverting this commit, it returns gitignore to the MFC master
version.
Merge MFC changes into ibdata-output
Keep ib_state_wrt addition, apply ruff formatting from master.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix ib_data.dat/ib.dat mismatch: update serial reader in
  m_data_input.f90 to match renamed writer
- Replace hard-coded unit 92 with newunit= for IB state file I/O
- Remove contradictory POSITION='append' from open statement
- Add ib_state_wrt requires ib validation in case_validator.py
  and m_checker.fpp
- Replace print*/return with s_mpi_abort in post_process error
  handling
- Make rank guard consistent: move proc_rank==0 to call site

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@MFlowCode MFlowCode deleted a comment from github-actions bot Mar 16, 2026
@MFlowCode MFlowCode deleted a comment from github-actions bot Mar 16, 2026
@github-actions
Copy link

github-actions bot commented Mar 16, 2026

Claude Code Review

Head SHA: 02f3448

Files changed:

  • 15 files
  • src/simulation/m_data_output.fpp, src/simulation/m_time_steppers.fpp, src/post_process/m_data_output.fpp, src/post_process/p_main.fpp, toolchain/mfc/params/definitions.py, toolchain/mfc/case_validator.py, and 9 more

Summary:

  • Adds ib_state_wrt parameter that streams IB force/torque/velocity/centroid state to a single binary file (D/ib_state.dat) during simulation
  • Post-process converts the binary into per-IB CSV text files (D/ib_<i>.txt)
  • Parameter correctly threaded through 4 required locations (definitions.py, m_global_parameters.fpp, m_start_up.fpp, case_validator.py)
  • ib_state_unit is initialized to -1 and s_open_ib_state_file has proper iostat= + s_mpi_abort error handling
  • Unrelated rename: ib.datib_data.dat for the per-timestep IB data file (simulation and post_process in sync)

Findings:

1. [Correctness] Stale force/torque written for moving_ibm==1 (1-way coupling)
src/simulation/m_time_steppers.fpp lines 636–645 — when any IB has moving_ibm > 0, moving_immersed_boundary_flag is true, and the else if (ib_state_wrt .and. s == nstage) branch is never reached. Inside s_propagate_immersed_boundaries, s_compute_ib_forces is only called for moving_ibm == 2 (two-way coupling). For moving_ibm == 1 (analytic/prescribed motion), forces are never computed, yet s_write_ib_state_file() on line 645 still writes patch_ib(i)%force / %torque — which remain at their initial/stale values. Output will be silently incorrect for 1-way moving IB cases.

Suggested fix — add an unconditional force-computation pass when ib_state_wrt is set:

if (moving_immersed_boundary_flag) then
    call s_propagate_immersed_boundaries(s)
end if
if (ib_state_wrt .and. s == nstage) then
    call s_compute_ib_forces(q_prim_vf, fluid_pp)
end if
if (proc_rank == 0 .and. ib_state_wrt .and. s == nstage) then
    call s_write_ib_state_file()
end if

(Guard with a flag if the double-computation for moving_ibm==2 is expensive.)

2. [Minor] Misleading comment
src/simulation/m_time_steppers.fpp line 638 — the comment reads "compute ib forces for fixed immersed boundaries" but the else if branch is reached whenever there are no moving IBs at all (i.e. moving_immersed_boundary_flag == .false.). It also implies the issue above: moving analytic IBs fall through to the force-write without computation.

3. [Testing] No regression test added
The PR checklist acknowledges that no test was added for ib_state_wrt. A single test in toolchain/mfc/test/cases.py that enables ib_state_wrt on an existing IB case and checks that the output files are produced would prevent regressions.

sbryngelson and others added 2 commits March 15, 2026 21:11
- Add iostat= + s_mpi_abort to the open call for consistent error
  handling with the rest of the file
- Initialize ib_state_unit = -1 as a sentinel to make misuse
  detectable at runtime

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sbryngelson sbryngelson marked this pull request as ready for review March 16, 2026 01:36
Copilot AI review requested due to automatic review settings March 16, 2026 01:36
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2026

Warning

Rate limit exceeded

@sbryngelson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a4c04928-fe47-4072-aee0-07b67a273bf5

📥 Commits

Reviewing files that changed from the base of the PR and between d8c479f and 02f3448.

📒 Files selected for processing (15)
  • docs/documentation/case.md
  • src/post_process/m_data_input.f90
  • src/post_process/m_data_output.fpp
  • src/post_process/m_global_parameters.fpp
  • src/post_process/m_mpi_proxy.fpp
  • src/post_process/m_start_up.fpp
  • src/post_process/p_main.fpp
  • src/simulation/m_checker.fpp
  • src/simulation/m_data_output.fpp
  • src/simulation/m_global_parameters.fpp
  • src/simulation/m_mpi_proxy.fpp
  • src/simulation/m_start_up.fpp
  • src/simulation/m_time_steppers.fpp
  • toolchain/mfc/case_validator.py
  • toolchain/mfc/params/definitions.py
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an optional output path for Immersed Boundary (IB) loads/state during simulation and a post-processing conversion routine, controlled by a new ib_state_wrt input parameter.

Changes:

  • Introduces ib_state_wrt as a new logical user input (toolchain + simulation + post_process).
  • Writes IB state/load records to a binary stream file (D/ib_state.dat) during simulation.
  • Post-process reads D/ib_state.dat and emits per-IB time-history text files; also renames serial IB marker output to ib_data.dat.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
toolchain/mfc/params/definitions.py Registers ib_state_wrt as an output logical parameter.
toolchain/mfc/case_validator.py Adds validation: ib_state_wrt requires ib.
src/simulation/m_time_steppers.fpp Opens/closes IB state output and writes records at RK final stage.
src/simulation/m_start_up.fpp Adds ib_state_wrt to the simulation namelist.
src/simulation/m_mpi_proxy.fpp Broadcasts ib_state_wrt across ranks.
src/simulation/m_global_parameters.fpp Declares/defaults ib_state_wrt.
src/simulation/m_data_output.fpp Implements binary ib_state.dat open/write/close routines.
src/simulation/m_checker.fpp Adds runtime prohibition: ib_state_wrt requires ib.
src/post_process/p_main.fpp Triggers IB state conversion at the end of post_process.
src/post_process/m_start_up.fpp Adds ib_state_wrt to the post_process namelist.
src/post_process/m_mpi_proxy.fpp Broadcasts ib_state_wrt across ranks.
src/post_process/m_global_parameters.fpp Declares/defaults ib_state_wrt.
src/post_process/m_data_output.fpp Reads ib_state.dat and writes per-IB formatted time histories.
src/post_process/m_data_input.f90 Updates serial IB marker filename to ib_data.dat.
docs/documentation/case.md Documents ib_state_wrt behavior.

Comment on lines +1526 to +1533
write (out_file, '(A,I0,A)') trim(file_loc)//'/ib_', i, '.txt'
open (newunit=iu_out(i), file=trim(out_file), form='formatted', status='replace', action='write', iostat=ios)
if (ios /= 0) then
call s_mpi_abort('Cannot open IB state output file: '//trim(out_file))
end if
write (iu_out(i), '(A)') &
'mytime fx fy fz Tau_x Tau_y Tau_z vx vy vz omega_x omega_y omega_z angle_x angle_y angle_z x_c y_c z_c'
end do
read (iu_in, iostat=ios) rec_time, rec_id, &
rec_force, rec_torque, rec_vel, rec_angular_vel, rec_angles, &
rec_centroid(1), rec_centroid(2), rec_centroid(3)
if (ios /= 0) exit
Comment on lines +262 to +273
impure subroutine s_open_ib_state_file
character(len=path_len + 2*name_len) :: file_loc
integer :: ios

write (file_loc, '(A)') 'ib_state.dat'
file_loc = trim(case_dir)//'/D/'//trim(file_loc)
open (newunit=ib_state_unit, file=trim(file_loc), &
form='unformatted', &
access='stream', &
status='replace', &
iostat=ios)
if (ios /= 0) call s_mpi_abort('Cannot open IB state output file: '//trim(file_loc))
Comment on lines +1524 to +1534
allocate (iu_out(num_ibs))
do i = 1, num_ibs
write (out_file, '(A,I0,A)') trim(file_loc)//'/ib_', i, '.txt'
open (newunit=iu_out(i), file=trim(out_file), form='formatted', status='replace', action='write', iostat=ios)
if (ios /= 0) then
call s_mpi_abort('Cannot open IB state output file: '//trim(out_file))
end if
write (iu_out(i), '(A)') &
'mytime fx fy fz Tau_x Tau_y Tau_z vx vy vz omega_x omega_y omega_z angle_x angle_y angle_z x_c y_c z_c'
end do

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 11.11111% with 56 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.13%. Comparing base (d8c479f) to head (02f3448).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/post_process/m_data_output.fpp 0.00% 34 Missing ⚠️
src/simulation/m_data_output.fpp 27.77% 13 Missing ⚠️
src/simulation/m_time_steppers.fpp 0.00% 5 Missing and 1 partial ⚠️
src/post_process/p_main.fpp 0.00% 1 Missing and 1 partial ⚠️
src/post_process/m_data_input.f90 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1302      +/-   ##
==========================================
- Coverage   45.27%   45.13%   -0.14%     
==========================================
  Files          70       70              
  Lines       20480    20546      +66     
  Branches     1952     1959       +7     
==========================================
+ Hits         9272     9274       +2     
- Misses      10084    10146      +62     
- Partials     1124     1126       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbryngelson sbryngelson merged commit 9b7a503 into MFlowCode:master Mar 16, 2026
70 of 72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants