Immersed Boundary force output#1292
Conversation
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.
Review Summary by QodoAdd immersed boundary force/torque output and post-processing
WalkthroughsDescription• Add binary output of immersed boundary force/torque data during simulation • Implement post-processing conversion from binary to CSV format per IB • Add ib_force_wrt parameter to control force output activation • Write force data at every RK final stage for both moving and fixed boundaries Diagramflowchart LR
A["Simulation Runtime"] -->|ib_force_wrt enabled| B["Write IB Forces to Binary"]
B -->|ib_force.dat| C["Post-Processing"]
C -->|Convert Binary| D["Per-IB CSV Files"]
D -->|ib_i.txt| E["Force/Torque Time Series"]
File Changes1. src/simulation/m_data_output.fpp
|
Code Review by Qodo
1. Uppercase keywords in open
|
There was a problem hiding this comment.
Pull request overview
Adds an optional immersed-boundary (IB) force/torque output pathway: simulation writes a single binary force log during runtime, and post_process converts it into per-IB time-history text files when ib_force_wrt is enabled.
Changes:
- Introduces new logical input parameter
ib_force_wrt(toolchain schema + simulation/post_process globals + MPI bcast + startup namelists). - Adds simulation-side binary output routines for IB force/torque records.
- Adds post_process conversion routine to split
D/ib_force.datinto per-IB time history files; updates user documentation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/mfc/params/definitions.py | Registers ib_force_wrt as an output parameter in the toolchain schema/registry. |
| src/simulation/m_time_steppers.fpp | Opens/closes the IB force file and triggers compute/write at RK final stage when enabled. |
| src/simulation/m_start_up.fpp | Adds ib_force_wrt to the simulation namelist. |
| src/simulation/m_mpi_proxy.fpp | Broadcasts ib_force_wrt to all ranks. |
| src/simulation/m_global_parameters.fpp | Declares/defaults ib_force_wrt. |
| src/simulation/m_data_output.fpp | Implements open/write/close routines for D/ib_force.dat; renames serial IB marker output file. |
| src/post_process/p_main.fpp | Calls the IB force conversion routine after the time-marching loop. |
| src/post_process/m_start_up.fpp | Adds ib_force_wrt to the post_process namelist. |
| src/post_process/m_mpi_proxy.fpp | Broadcasts ib_force_wrt to all ranks. |
| src/post_process/m_global_parameters.fpp | Declares/defaults ib_force_wrt. |
| src/post_process/m_data_output.fpp | Adds s_write_ib_force_files() conversion from binary stream to per-IB formatted files. |
| docs/documentation/case.md | Documents ib_force_wrt behavior and output. |
| allocate (iu_out(num_ibs)) | ||
| do i = 1, num_ibs | ||
| write (out_file, '(A,I0,A)') trim(file_loc)//'/ib_', i, '.txt' |
There was a problem hiding this comment.
allocate(iu_out(num_ibs)) will crash if ib_force_wrt is enabled but num_ibs is unset/invalid (it defaults to dflt_int) or if ib is false. Add a guard (e.g., require ib and num_ibs > 0) before allocating/looping, and fail fast with a clear error if ib_force_wrt is inconsistent with the IB configuration.
| open (newunit=iu_in, file=trim(in_file), form='unformatted', access='stream', & | ||
| status='old', action='read', iostat=ios) | ||
| if (ios /= 0) then | ||
| print *, 'ERROR: cannot open input file: ', trim(in_file), ' iostat=', ios | ||
| return | ||
| end if |
There was a problem hiding this comment.
On failure to open the input/output files this routine print *s an error and returns. Elsewhere in post_process, missing/invalid required files typically trigger s_mpi_abort, which avoids silent partial outputs and keeps all ranks in a consistent state. Consider using s_mpi_abort here as well (or otherwise propagate a failure status to the caller).
| 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 | ||
| print *, 'ERROR: cannot open output file: ', trim(out_file), ' iostat=', ios | ||
| close (iu_in) | ||
| do rec_id = 1, i - 1 | ||
| close (iu_out(rec_id)) | ||
| end do | ||
| deallocate (iu_out) | ||
| return | ||
| end if | ||
| write (iu_out(i), '(A)') 'mytime fx fy fz Tau_x Tau_y Tau_z' | ||
| end do |
There was a problem hiding this comment.
The PR description mentions conversion to CSV, but the post-processing output is written as space-separated .txt files with a non-CSV header. Either emit actual CSVs (comma-separated and preferably .csv extension) or update the PR description/docs to match the actual output format.
| for n in ["parallel_io", "file_per_process", "run_time_info", "prim_vars_wrt", | ||
| "cons_vars_wrt", "fft_wrt"]: | ||
| "cons_vars_wrt", "fft_wrt", "ib_force_wrt"]: | ||
| _r(n, LOG, {"output"}) |
There was a problem hiding this comment.
This new parameter is correctly added to the registry, but the toolchain’s fallback namelist parameter sets (used when Fortran sources aren’t available, e.g. Homebrew installs) are defined in toolchain/mfc/params/namelist_parser.py and currently do not include ib_force_wrt for simulation/post_process. Please regenerate/update those fallback sets so ib_force_wrt is accepted in source-less installations too.
| write (t_step_dir, '(a,i0,a,i0)') trim(case_dir)//'/p_all/p', & | ||
| proc_rank, '/', time_step | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib.dat' | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib_data.dat' |
There was a problem hiding this comment.
s_write_serial_ib_data now writes ib_data.dat, but the post-process reader still looks for ib.dat (see src/post_process/m_data_input.f90). This will break post-processing (and possibly restarts) for serial I/O IB marker data. Either keep the original filename, or update the post-process input logic to accept the new name (ideally supporting both for backward compatibility).
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib_data.dat' | |
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib.dat' |
|
|
||
| write (file_loc, '(A)') 'ib_force.dat' | ||
| file_loc = trim(case_dir)//'/D/'//trim(file_loc) | ||
| open (92, FILE=trim(file_loc), & | ||
| FORM='unformatted', & | ||
| ACCESS='stream', & | ||
| STATUS='replace', & | ||
| POSITION='append') |
There was a problem hiding this comment.
The IB force output uses a hard-coded unit number (92) for open/write/close. This can collide with other units already used in this module (e.g., integrals use i+70, so num_integrals>=22 will also use unit 92), leading to failed opens or writing/closing the wrong file. Use open(newunit=...) and store the resulting unit in a module variable used by the write/close routines (and consider checking iostat on open).
| write (file_loc, '(A)') 'ib_force.dat' | |
| file_loc = trim(case_dir)//'/D/'//trim(file_loc) | |
| open (92, FILE=trim(file_loc), & | |
| FORM='unformatted', & | |
| ACCESS='stream', & | |
| STATUS='replace', & | |
| POSITION='append') | |
| integer, save :: ib_force_unit = -1 | |
| integer :: istat | |
| write (file_loc, '(A)') 'ib_force.dat' | |
| file_loc = trim(case_dir)//'/D/'//trim(file_loc) | |
| open (newunit=ib_force_unit, FILE=trim(file_loc), & | |
| FORM='unformatted', & | |
| ACCESS='stream', & | |
| STATUS='replace', & | |
| POSITION='append', & | |
| iostat=istat) | |
| if (istat /= 0) then | |
| write (*,*) 'Error opening IB force file: ', trim(file_loc), ' iostat = ', istat | |
| end if |
| call s_open_run_time_information_file() | ||
| end if | ||
|
|
||
| ! Opening and writing the header of the ib data file |
There was a problem hiding this comment.
This comment says the routine is "opening and writing the header" of the IB force file, but s_open_ib_force_file only opens the stream and doesn't write any header record. Please either write an actual header (and update the post-process reader accordingly) or adjust the comment to avoid misleading future maintainers.
| ! Opening and writing the header of the ib data file | |
| ! Opening the IB force data file |
| call s_close_run_time_information_file() | ||
| end if | ||
|
|
||
| ! Writing the footer of and closing the IB data file |
There was a problem hiding this comment.
This comment says a footer is written for the IB force file, but s_close_ib_force_file only closes the unit and does not write any footer record. Please either implement a footer (and adjust the post-process reader) or update the comment to match the actual behavior.
| ! Writing the footer of and closing the IB data file | |
| ! Closing the IB data file |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| impure subroutine s_open_ib_force_file | ||
| character(LEN=path_len + 2*name_len) :: file_loc | ||
|
|
||
| write (file_loc, '(A)') 'ib_force.dat' | ||
| file_loc = trim(case_dir)//'/D/'//trim(file_loc) | ||
| open (92, FILE=trim(file_loc), & | ||
| FORM='unformatted', & | ||
| ACCESS='stream', & | ||
| STATUS='replace', & | ||
| POSITION='append') |
There was a problem hiding this comment.
1. Uppercase keywords in open 📘 Rule violation ✓ Correctness
New Fortran code uses uppercase keywords (e.g., LEN, FILE, FORM, ACCESS, STATUS, POSITION) and non-2-space indentation, violating the required style conventions. This reduces consistency and can hinder maintainability across the codebase.
Agent Prompt
## Issue description
New Fortran code violates the required style conventions by using uppercase keywords (e.g., `LEN=`, `FILE=`, `FORM=`) and non-2-space indentation.
## Issue Context
Project style requires 2-space indentation and lowercase Fortran keywords for new/modified code.
## Fix Focus Areas
- src/simulation/m_data_output.fpp[260-269]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| impure subroutine s_write_ib_force_files() | ||
|
|
||
| character(len=len_trim(case_dir) + 4*name_len) :: in_file, out_file, file_loc | ||
| integer :: iu_in, ios, i, rec_step, rec_id | ||
| integer, allocatable, dimension(:) :: iu_out | ||
| real(wp) :: rec_time | ||
| real(wp), dimension(3) :: rec_force, rec_torque | ||
|
|
There was a problem hiding this comment.
2. s_write_ib_force_files indentation 📘 Rule violation ✓ Correctness
The newly added post-processing routine is not formatted with the required 2-space indentation. This violates the mandated Fortran style conventions and reduces consistency across modules.
Agent Prompt
## Issue description
The newly added `s_write_ib_force_files` routine does not follow the required 2-space indentation convention.
## Issue Context
Project style requires 2-space indentation for new/modified Fortran code.
## Fix Focus Areas
- src/post_process/m_data_output.fpp[1503-1553]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| write (t_step_dir, '(A,I0,A,I0)') trim(case_dir)//'/p_all' | ||
| write (t_step_dir, '(a,i0,a,i0)') trim(case_dir)//'/p_all/p', & | ||
| proc_rank, '/', time_step | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib.dat' | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib_data.dat' | ||
|
|
||
| open (2, FILE=trim(file_path), & | ||
| FORM='unformatted', & |
There was a problem hiding this comment.
3. Ib marker file rename breaks post_process 🐞 Bug ✓ Correctness
Simulation serial IB marker output was renamed to ib_data.dat, but post_process still looks for ib.dat and aborts if it’s missing. This will break post-processing for cases that rely on the per-timestep serial IB marker files.
Agent Prompt
### Issue description
Simulation now writes per-timestep serial IB marker files as `ib_data.dat`, but post-processing still requires `ib.dat` and aborts if not found.
### Issue Context
This breaks post-processing for IB cases in the serial-per-timestep path (`p_all/p<rank>/<tstep>/...`).
### Fix Focus Areas
- src/post_process/m_data_input.f90[164-208]
- src/simulation/m_data_output.fpp[1085-1093]
### Suggested approach
- In `s_read_ib_data_files`, when `parallel_io` is false, first try `ib_data.dat`; if missing, fall back to `ib.dat` (or switch fully to `ib_data.dat` if backward compatibility isn’t needed).
- Keep the `parallel_io` branch reading `restart_data/.../ib.dat` unchanged unless you also renamed that output.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| ! check if any IBMS are moving, and if so, update the markers, ghost points, levelsets, and levelset norms | ||
| if (moving_immersed_boundary_flag) then | ||
| call s_propagate_immersed_boundaries(s) | ||
| ! compute ib forces for fixed immersed boundaries if requested for output | ||
| else if (ib_force_wrt .and. s == nstage) then | ||
| call s_compute_ib_forces(q_prim_vf, fluid_pp) | ||
| end if | ||
|
|
||
| ! Write IB forces to file if requested and at the RK final stage | ||
| if (ib_force_wrt .and. s == nstage) then | ||
| call s_write_ib_force_file(t_step) | ||
| end if |
There was a problem hiding this comment.
4. Forces may be stale/zero 🐞 Bug ✓ Correctness
When any IB is moving, the new logic skips s_compute_ib_forces in the main RK loop, yet still writes patch_ib(i)%force/torque for all IBs. This can produce incorrect force/torque output for fixed IBs in mixed cases and for moving_ibm==1 (analytic motion) where forces are never computed.
Agent Prompt
### Issue description
`m_time_steppers` writes IB forces at the final RK stage whenever `ib_force_wrt` is enabled, but it only computes forces in the non-moving case (or for `moving_ibm==2`). This can leave `patch_ib(:)%force/torque` unset/stale in mixed moving/fixed IB cases or for `moving_ibm==1`.
### Issue Context
`moving_immersed_boundary_flag` becomes true if *any* IB has `moving_ibm /= 0`, which prevents the fixed-IB compute path from running.
### Fix Focus Areas
- src/simulation/m_time_steppers.fpp[634-646]
- src/simulation/m_time_steppers.fpp[835-856]
### Suggested approach
- At `s == nstage` and `ib_force_wrt`, call `s_compute_ib_forces(q_prim_vf, fluid_pp)` unconditionally before `s_write_ib_force_file(t_step)`.
- Optionally avoid double-computation by tracking whether forces were already computed during this stage (e.g., a stage-local flag set by `s_propagate_immersed_boundaries`).
- Keep `s_propagate_immersed_boundaries` behavior for two-way coupling, but ensure final-stage output always reflects current state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (proc_rank == 0 .and. ib_force_wrt) then | ||
| call s_write_ib_force_files() | ||
| end if |
There was a problem hiding this comment.
5. Post_process can crash 🐞 Bug ⛯ Reliability
post_process calls s_write_ib_force_files() whenever ib_force_wrt is true, even if ib is false and num_ibs remains at its sentinel default. This can lead to invalid allocation sizes (allocate(iu_out(num_ibs))) and crashes for misconfigured inputs.
Agent Prompt
### Issue description
`post_process/p_main.fpp` calls the IB force conversion whenever `ib_force_wrt` is enabled, but the conversion allocates arrays based on `num_ibs`, which can be left at a sentinel value if `ib` is false or inputs are inconsistent.
### Issue Context
Current behavior can crash on `allocate(iu_out(num_ibs))`.
### Fix Focus Areas
- src/post_process/p_main.fpp[96-98]
- src/post_process/m_data_output.fpp[1503-1524]
- src/post_process/m_global_parameters.fpp[497-499]
### Suggested approach
- Change the call site to:
- `if (proc_rank == 0 .and. ib .and. ib_force_wrt) call s_write_ib_force_files()`
- Inside `s_write_ib_force_files`, add a defensive check:
- if `num_ibs <= 0` then print a clear error and return (or abort).
- Optionally also check the existence of the input file before allocating output units.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
📝 WalkthroughWalkthroughThis pull request introduces a new immersed boundary (IB) force output feature controlled by a logical parameter 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/simulation/m_data_output.fpp (2)
1157-1169: Flush the force stream if this output is meant to survive abnormal exits.These records are written every time step, but they are only guaranteed to hit disk on
close. A CFL abort or walltime kill can drop the tail ofib_force.dat, which weakens the value of the new live force output.Suggested hardening
impure subroutine s_write_ib_force_file(time_step) integer, intent(in) :: time_step if (proc_rank == 0) then block integer :: i do i = 1, num_ibs write (92) time_step, mytime, i, patch_ib(i)%force, patch_ib(i)%torque end do end block + flush (92) end if end subroutine s_write_ib_force_file
260-269: Replace the repeated unit number with a named constant.Hard-coding
92in open/write/close makes this easy to desynchronize later and harder to audit for unit collisions. A module-level parameter keeps the file handle contract in one place.Refactor sketch
Add near the module-scope declarations:
integer, parameter :: ib_force_unit = 92Then update the call sites:
- open (92, FILE=trim(file_loc), & + open (ib_force_unit, FILE=trim(file_loc), & FORM='unformatted', & ACCESS='stream', & STATUS='replace', & POSITION='append')- write (92) time_step, mytime, i, patch_ib(i)%force, patch_ib(i)%torque + write (ib_force_unit) time_step, mytime, i, patch_ib(i)%force, patch_ib(i)%torque- close (92) + close (ib_force_unit)Also applies to: 1164-1164, 1940-1943
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6147423-7d2b-46ba-bef6-ca623f730168
📒 Files selected for processing (12)
docs/documentation/case.mdsrc/post_process/m_data_output.fppsrc/post_process/m_global_parameters.fppsrc/post_process/m_mpi_proxy.fppsrc/post_process/m_start_up.fppsrc/post_process/p_main.fppsrc/simulation/m_data_output.fppsrc/simulation/m_global_parameters.fppsrc/simulation/m_mpi_proxy.fppsrc/simulation/m_start_up.fppsrc/simulation/m_time_steppers.fpptoolchain/mfc/params/definitions.py
| open (newunit=iu_in, file=trim(in_file), form='unformatted', access='stream', & | ||
| status='old', action='read', iostat=ios) | ||
| if (ios /= 0) then | ||
| print *, 'ERROR: cannot open input file: ', trim(in_file), ' iostat=', ios | ||
| return |
There was a problem hiding this comment.
Fail on bad force-file input.
ios /= 0 here treats corruption as EOF, and the rec_id guard silently drops records that do not match the configured num_ibs. The open-failure paths above also just return. That can leave incomplete per-IB histories while post-process still exits successfully. Only exit on ios < 0; abort on positive ios or an out-of-range ID.
🛠️ Suggested hard-fail read loop
do
read (iu_in, iostat=ios) rec_step, rec_time, rec_id, rec_force, rec_torque
- if (ios /= 0) exit ! EOF (<0) or read error (>0)
- if (rec_id >= 1 .and. rec_id <= num_ibs) then
- write (iu_out(rec_id), '(7(ES24.16E3,1X))') rec_time, &
- rec_force(1), rec_force(2), rec_force(3), &
- rec_torque(1), rec_torque(2), rec_torque(3)
- end if
+ if (ios < 0) exit
+ if (ios > 0) call s_mpi_abort('Failed while reading '//trim(in_file)//'.')
+ if (rec_id < 1 .or. rec_id > num_ibs) then
+ call s_mpi_abort('IB force record id is out of range for current num_ibs.')
+ end if
+ write (iu_out(rec_id), '(7(ES24.16E3,1X))') rec_time, &
+ rec_force(1), rec_force(2), rec_force(3), &
+ rec_torque(1), rec_torque(2), rec_torque(3)
end doAlso applies to: 1524-1532, 1537-1544
| 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) |
There was a problem hiding this comment.
This is whitespace-delimited text, not CSV.
The new files use .txt, a space-separated header, and whitespace-separated rows. If this feature is supposed to be the CSV conversion step described in the PR, downstream tooling will not parse it as advertised.
Based on learnings: Flag modifications to public subroutine signatures, parameter defaults, or output formats - they affect downstream users.
Also applies to: 1534-1543
| write (t_step_dir, '(a,i0,a,i0)') trim(case_dir)//'/p_all/p', & | ||
| proc_rank, '/', time_step | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib.dat' | ||
| write (file_path, '(A,I0,A)') trim(t_step_dir)//'/ib_data.dat' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all references to ib_data.dat and ib.dat in the repo
echo "=== All references to ib_data.dat and ib.dat ==="
rg -n --no-heading -C2 '\bib(?:_data)?\.dat\b'Repository: MFlowCode/MFC
Length of output: 2384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Examine the file around line 1089 and find the parallel I/O path
echo "=== Context around line 1089 in m_data_output.fpp ==="
sed -n '1080,1100p' src/simulation/m_data_output.fpp
echo -e "\n=== Search for parallel IB output (ib.dat) ==="
rg -n 'ib\.dat' src/simulation/m_data_output.fpp -C3Repository: MFlowCode/MFC
Length of output: 2082
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's conditional logic for serial vs parallel IB output
echo "=== Search for s_write_serial_ib_data and related routines ==="
rg -n 's_write.*ib' src/simulation/m_data_output.fpp -A5Repository: MFlowCode/MFC
Length of output: 2628
Serial IB output filename breaks post-process reader compatibility.
s_write_serial_ib_data() writes ib_data.dat (line 1089), but src/post_process/m_data_input.f90 reads ib.dat for serial mode (lines 166–167). This mismatch will cause post-process to fail with "file not found" for IB data in serial-I/O mode. Either revert the serial filename to ib.dat to match the reader, or update the post-process reader to expect ib_data.dat in serial mode.
| ib, num_ibs, patch_ib, & | ||
| ib_force_wrt, & | ||
| fluid_pp, bub_pp, probe_wrt, prim_vars_wrt, & |
There was a problem hiding this comment.
Reject ib_force_wrt unless IB is enabled.
s_initialize_modules() only initializes IBM under if (ib), but the force-output path in src/simulation/m_time_steppers.fpp:467-469 and src/simulation/m_time_steppers.fpp:637-641 is gated by ib_force_wrt alone. This new namelist entry therefore exposes a configuration that can reach IB force routines with uninitialized IB state.
🛡️ Suggested validation hook
call s_check_inputs_common()
call s_check_inputs()
+ if (ib_force_wrt .and. .not. ib) then
+ call s_mpi_abort('ib_force_wrt requires ib = T. Exiting.')
+ end if| ! Opening and writing the header of the ib data file | ||
| if (proc_rank == 0 .and. ib_force_wrt) then | ||
| call s_open_ib_force_file() | ||
| end if |
There was a problem hiding this comment.
Gate the IB force file lifetime on ib too.
With ib_force_wrt = T and ib = F, Line 467 still opens and Line 1073 still closes the dedicated IB file, but the only write path is inside if (ib) at Lines 634-645. That leaves a misleading empty artifact unless this combination is rejected upstream.
Suggested fix
- if (proc_rank == 0 .and. ib_force_wrt) then
+ if (proc_rank == 0 .and. ib .and. ib_force_wrt) then
call s_open_ib_force_file()
end if
@@
- if (proc_rank == 0 .and. ib_force_wrt) then
+ if (proc_rank == 0 .and. ib .and. ib_force_wrt) then
call s_close_ib_force_file()
end ifBased on learnings, "Every new parameter must be added in at least 3 places: (1) toolchain/mfc/params/definitions.py, (2) Fortran variable declaration in src//m_global_parameters.fpp, (3) Fortran namelist in src//m_start_up.fpp, and optionally (4) toolchain/mfc/case_validator.py if the parameter has physics constraints".
Also applies to: 1072-1075
| ! compute ib forces for fixed immersed boundaries if requested for output | ||
| else if (ib_force_wrt .and. s == nstage) then | ||
| call s_compute_ib_forces(q_prim_vf, fluid_pp) | ||
| end if | ||
|
|
||
| ! Write IB forces to file if requested and at the RK final stage | ||
| if (ib_force_wrt .and. s == nstage) then | ||
| call s_write_ib_force_file(t_step) |
There was a problem hiding this comment.
Moving-IB force output can be stale or missing.
When moving_immersed_boundary_flag is true, Lines 636-641 bypass the fixed-IB force path and rely on s_propagate_immersed_boundaries(). Later in this file, that routine only calls s_compute_ib_forces() for moving_ibm == 2, so one-way moving IBs—and mixed fixed + one-way cases—reach Lines 643-645 with old or never-populated patch_ib(:)%force/torque.
Suggested fix
if (moving_immersed_boundary_flag) then
call s_propagate_immersed_boundaries(s)
- ! compute ib forces for fixed immersed boundaries if requested for output
- else if (ib_force_wrt .and. s == nstage) then
- call s_compute_ib_forces(q_prim_vf, fluid_pp)
end if
- ! Write IB forces to file if requested and at the RK final stage
if (ib_force_wrt .and. s == nstage) then
+ call s_compute_ib_forces(q_prim_vf, fluid_pp)
call s_write_ib_force_file(t_step)
end if
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1292 +/- ##
==========================================
- Coverage 44.95% 44.83% -0.12%
==========================================
Files 70 70
Lines 20503 20560 +57
Branches 1946 1953 +7
==========================================
+ Hits 9217 9219 +2
- Misses 10164 10217 +53
- Partials 1122 1124 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
The add binary output of IB force data during the simulation run, and conversion to a CSV file in post processing. Including the parameter ib_force_wrt="T" will activate the output routines.
Type of change
Testing
How did you test your changes?
ran lint, format, test -a on a MacBookPro (10 core) and HiperGator GPUs.
Only UUID: 6F35CD77 (3D Bubbles test) failed for tolerance. The modifications did not touch any of the bubble code and this failure has been seen before the modification.
Checklist
See the developer guide for full coding standards.
GPU changes (expand if you modified
src/simulation/)AI code reviews
Reviews are not triggered automatically. To request a review, comment on the PR:
@coderabbitai review— incremental review (new changes only)@coderabbitai full review— full review from scratch/review— Qodo review/improve— Qodo code suggestions