fix(ibm): fix IB restart and state output for adaptive-dt (cfl_dt) runs#1382
fix(ibm): fix IB restart and state output for adaptive-dt (cfl_dt) runs#1382mrvandenboom wants to merge 1 commit intoMFlowCode:masterfrom
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the immersed boundary (IB) restart-state persistence logic in 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd6de904-3d92-43c3-b561-e70688cc245c
📒 Files selected for processing (1)
src/simulation/m_start_up.fpp
| end if | ||
| call s_ibm_setup() | ||
| if (t_step_start == 0) then | ||
| if (t_step_start == 0 .or. (cfl_dt .and. n_start == 0)) then |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for how t_step_start interacts with cfl_dt / n_start during input assignment and validation
rg -nP --type=fortran -C3 '\bt_step_start\b' src/common src/simulation | rg -nP -C3 '(cfl_dt|n_start|dflt|default|assign)'
rg -nP --type=fortran -C3 '\bn_start\b' src/common src/simulation | rg -nP -C3 '(cfl_dt|t_step_start|dflt|default|assign)'
# Default value for t_step_start
ast-grep --pattern 't_step_start = $_'Repository: MFlowCode/MFC
Length of output: 325
🏁 Script executed:
# Find default value of t_step_start in module declarations
rg -nP --type=fortran 't_step_start\s*=' src/common src/simulation | grep -i 'default\|init\|=\s*0'
# Look at context around line 923 in m_start_up.fpp
sed -n '915,935p' src/simulation/m_start_up.fpp
# Find how n_start and t_step_start are used in cfl_dt logic
rg -nP --type=fortran -B5 -A5 'if.*cfl_dt' src/simulation/m_start_up.fpp | head -60
# Search for input assignment routines
fd -e fpp -e f90 'assign.*default\|assign.*input' src/Repository: MFlowCode/MFC
Length of output: 3582
🏁 Script executed:
# Find where t_step_start is declared and initialized
rg -nP --type=fortran 'integer.*::.*t_step_start|t_step_start\s*=' src/common src/simulation | head -20
# Find where n_start is declared and initialized
rg -nP --type=fortran 'integer.*::.*n_start|n_start\s*=' src/common src/simulation | head -20
# Look for input assignment/validation routines
fd -e fpp -e f90 . src/common src/simulation | xargs rg -l 's_assign_default\|s_read.*input\|s_initialize.*input' | head -5
# Check if there's any synchronization logic between cfl_dt mode and these variables
rg -nP --type=fortran -B3 -A3 'n_start.*t_step_start|t_step_start.*n_start' src/Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Search more carefully for declarations and defaults
rg -nP 't_step_start' src/simulation/m_start_up.fpp | head -10
# Look for module variable declarations (usually at top)
head -100 src/simulation/m_start_up.fpp | grep -n 'integer\|real\|logical'
# Check global variables module
rg -nP 't_step_start' src/common/m_global_variables.fpp
# Check Python input case
rg -nP 't_step_start\|n_start' toolchain/mfc/state.py | head -20Repository: MFlowCode/MFC
Length of output: 1261
🏁 Script executed:
# Look at the top of m_start_up.fpp for module declarations and USE statements
head -50 src/simulation/m_start_up.fpp
# Find where these globals are declared - likely in another module
rg -nP 'integer.*t_step_start|integer.*n_start' src/ --type=fortran | grep -v 'intent\|character\|write'
# Check if they're in a global_variables type module
fd -e fpp -e f90 . src/common | xargs rg -l 't_step_start\|n_start'Repository: MFlowCode/MFC
Length of output: 1084
🏁 Script executed:
# Find m_global_parameters module
fd -e fpp -e f90 m_global_parameters src/
# Search for t_step_start and n_start declarations
rg -nP 't_step_start|n_start' src/common/m_global_parameters.fpp | head -30Repository: MFlowCode/MFC
Length of output: 270
🏁 Script executed:
# Search for t_step_start and n_start in the simulation module
rg -nP 't_step_start|n_start' src/simulation/m_global_parameters.fpp | head -50
# Look for initialization/default values
rg -nP -B2 -A2 't_step_start\s*=|n_start\s*=' src/simulation/m_global_parameters.fppRepository: MFlowCode/MFC
Length of output: 481
🏁 Script executed:
# Find what dflt_int is set to
rg -nP 'dflt_int\s*=' src/simulation/m_global_parameters.fpp
# Also check if there's input reading/assignment that explicitly sets these for restart cases
rg -nP -B5 -A5 's_read_input_file|s_assign_default' src/simulation/m_start_up.fpp | head -40Repository: MFlowCode/MFC
Length of output: 1614
🏁 Script executed:
# Search for dflt_int definition across all source files
rg -nP 'parameter.*dflt_int|dflt_int\s*=\s*' src/ --type=fortran | head -20
# Look at s_read_input_file to understand input assignment
sed -n '75,150p' src/simulation/m_start_up.fpp
# Search for where t_step_start and n_start are set from input
rg -nP 't_step_start\s*=|n_start\s*=' src/simulation/m_start_up.fppRepository: MFlowCode/MFC
Length of output: 3817
🏁 Script executed:
# Find dflt_int definition - likely in parameters module
rg -nP 'parameter.*dflt_int' src/ --type=fortran
# Also check what value it gets set to
rg -nP 'dflt_int' src/pre_process/m_global_parameters.fpp | head -5Repository: MFlowCode/MFC
Length of output: 231
🏁 Script executed:
# Search for dflt_int parameter definition
rg -nP 'integer.*parameter.*dflt_int|dflt_int.*=' src/simulation/m_global_parameters.fpp | grep -v 't_step\|n_start'
# Check parameter declarations at the top of the module
head -200 src/simulation/m_global_parameters.fpp | grep -n 'parameter\|dflt'Repository: MFlowCode/MFC
Length of output: 2842
🏁 Script executed:
# Search directly for dflt_int as a named constant
rg -nP 'dflt_int' src/simulation/m_global_parameters.fpp | grep 'parameter\|integer.*dflt'
# Also check if it's imported from elsewhere or defined in a common parameters file
rg -nP 'dflt_int' src/ --type=fortran | grep -i 'parameter\|=-\|=0'Repository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Look at the actual definitions in the module - search broadly
rg -nP 'dflt' src/simulation/m_global_parameters.fpp | head -20
# Check if it's a Fypp macro
grep -n 'dflt_int' src/simulation/m_global_parameters.fpp | head -10Repository: MFlowCode/MFC
Length of output: 1092
🏁 Script executed:
# Check the includes at the top - case.fpp and macros.fpp
cat src/simulation/case.fpp | head -50
# Also check macros
cat src/common/include/macros.fpp | grep -n 'dflt'Repository: MFlowCode/MFC
Length of output: 113
🏁 Script executed:
# Look for case.fpp in include directory
find src -name 'case.fpp' -o -name 'macros.fpp'
# Search for dflt_int definition in include files
find src -name '*.fpp' -o -name '*.fpp' | xargs rg -l 'dflt_int'
# Check if dflt_int is set to a specific value
rg -nP '#:set|dflt_int\s*:=' src/Repository: MFlowCode/MFC
Length of output: 35019
🏁 Script executed:
# Search for dflt_int definition in macros.fpp
rg -nP 'dflt_int' src/common/include/macros.fpp
# Also check case.fpp
rg -nP 'dflt_int' src/common/include/case.fppRepository: MFlowCode/MFC
Length of output: 39
🏁 Script executed:
# Search for dflt_int across the entire codebase more carefully
rg 'dflt_int' src/ --type=fortran -C1 | grep -E '(parameter|integer|=)' | head -20
# Look at what the actual value would be - search in Python/config
rg 'dflt_int|default.*int' toolchain/ | head -10
# Check if it's defined via preprocessing - look at the actual generated file
rg -nP 'integer.*parameter' src/simulation/m_global_parameters.fpp | head -20Repository: MFlowCode/MFC
Length of output: 1780
Mode-exclusive check needed for initial IBM file writes in cfl_dt restart.
The disjunction at line 923 is mode-agnostic: t_step_start == 0 .or. (cfl_dt .and. n_start == 0) short-circuits on the first clause. When restarting with cfl_dt=true and n_start > 0, if t_step_start remains at its default value, the first clause evaluates true and the branch overwrites ib_data_0/ib_state_0 files with initial state, clobbering the restart data.
The restart read logic (lines 920–922) correctly branches on cfl_dt: use n_start when enabled, else use t_step_start. The write condition must follow the same pattern:
🛠️ Proposed fix
- if (t_step_start == 0 .or. (cfl_dt .and. n_start == 0)) then
+ if ((cfl_dt .and. n_start == 0) .or. &
+ (.not. cfl_dt .and. t_step_start == 0)) then
call s_write_ib_data_file(0)
call s_write_ib_state_file(0)
end if📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (t_step_start == 0 .or. (cfl_dt .and. n_start == 0)) then | |
| if ((cfl_dt .and. n_start == 0) .or. & | |
| (.not. cfl_dt .and. t_step_start == 0)) then |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1382 +/- ##
==========================================
- Coverage 64.76% 64.76% -0.01%
==========================================
Files 71 71
Lines 18713 18715 +2
Branches 1549 1549
==========================================
+ Hits 12119 12120 +1
- Misses 5638 5640 +2
+ Partials 956 955 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Description
Summarize your changes and the motivation behind them.
Fixes issue where MFC does not find the restart file for adaptive time stepping runs. The issue was the conditional variable that only is present when doing fixed time stepping. Added a conditional statement to work with both time stepping modes.
Also changed the save points to occur on "save count" vs "time step".
Type of change
Testing
Ran single test case.
Checklist
GPU changes (expand if you modified
src/simulation/)AI code reviews
@coderabbitai review
@claude full review