Fix post_process non-MPI builds never reading input file#1272
Conversation
In s_initialize_mpi_domain, the entire body including s_assign_default_values_to_user_inputs(), s_read_input_file(), and s_check_input_file() was guarded by #ifdef MFC_MPI. Non-MPI builds therefore ran with all parameters at Fortran defaults (case_dir blank, m/n/p = 0), causing a misleading "Time-step folder missing" error. pre_process and simulation already call these unconditionally; the individual MPI functions (s_mpi_bcast_user_inputs, etc.) handle the #ifdef MFC_MPI guard internally as stubs when MPI is absent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move three Fortran @:PROHIBIT checks that test case parameters against build configuration flags into case_validator.py, so they fire before any binary is invoked rather than at Fortran runtime: - parallel_io = T requires --mpi (all stages, via check_build_flags) - precision = 2 requires no --single (post_process, in check_output_format) - 3D cylindrical + p > 0 requires no --single (simulation only, via check_geometry_precision_simulation) CFG() from state.py carries the active build config (mpi, single, mixed). During standalone './mfc.sh validate', CFG() holds defaults (mpi=T, single=F) so these checks do not produce false positives. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The three build-flag compatibility checks moved to case_validator.py in the previous commit no longer need Fortran-side enforcement. Remove them and their now-empty subroutines: - pre_process/m_checker.fpp: s_check_parallel_io (parallel_io + --no-mpi) - simulation/m_checker.fpp: s_check_inputs_geometry_precision (3D cyl + --single) - post_process/m_checker.fpp: s_check_inputs_output_format (precision=2 + --single) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
self.get('p', 0) always returns an int (0 when absent), so the
p is not None check was always True and never needed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude Code ReviewHead SHA: Files changed: 6
Summary
Findings[Minor] call s_mpi_bcast_user_inputs()
call s_initialize_parallel_io()
call s_mpi_decompose_computational_domain()
call s_check_inputs_fft() ! ← now always calledThe first three functions are documented as having internal [Design note] Removed Fortran [Nit] Comment typo in
"not truly executed in parallel" is the negated condition (i.e., serial run). The comment is logically correct but mirrors the old pre-existing phrasing that is slightly awkward. Not a blocker. No blocking issues found. The core bug fix is sound and the moved checks are logically equivalent. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR refactors input validation by removing runtime checks from Fortran modules and consolidating them into the Python toolchain validator. Four validation subroutines are deleted from three Fortran modules (s_check_inputs_output_format, s_check_parallel_io, s_check_inputs_geometry_precision), while two new validation methods are added to CaseValidator (check_build_flags, check_geometry_precision_simulation). The MPI initialization conditional guard in m_start_up.fpp is also removed, making the MPI code path unconditional. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where post_process built without MPI (--no-mpi) never read the input file, leaving all parameters at their Fortran default values and producing misleading error messages. The fix aligns post_process with the existing pattern used by pre_process and simulation.
Changes:
- Remove outer
#ifdef MFC_MPIguard ins_initialize_mpi_domainso input file reading occurs in all builds - Migrate three build-flag compatibility checks from Fortran to Python case validator for earlier error detection
- Update documentation lint skip list for the new build-flag check methods
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/post_process/m_start_up.fpp | Remove outer #ifdef MFC_MPI guard in s_initialize_mpi_domain; update comment to say "post-process" instead of "simulation" |
| toolchain/mfc/case_validator.py | Add check_build_flags() and check_geometry_precision_simulation() methods; add precision == 2 check to check_output_format() |
| src/pre_process/m_checker.fpp | Remove s_check_parallel_io() subroutine (migrated to Python) |
| src/post_process/m_checker.fpp | Remove s_check_inputs_output_format() subroutine (migrated to Python) |
| src/simulation/m_checker.fpp | Remove s_check_inputs_geometry_precision() subroutine and call (migrated to Python) |
| toolchain/mfc/lint_docs.py | Add new build-flag check methods to skip list as they have no physics documentation |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1272 +/- ##
==========================================
- Coverage 44.05% 44.04% -0.02%
==========================================
Files 70 70
Lines 20496 20492 -4
Branches 1991 1992 +1
==========================================
- Hits 9029 9025 -4
+ Misses 10328 10327 -1
- Partials 1139 1140 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
In
s_initialize_mpi_domaininsrc/post_process/m_start_up.fpp, the entire subroutine body was wrapped in#ifdef MFC_MPI. Non-MPI builds (--no-mpi) therefore skipped all input reading, leaving every parameter at its Fortran default —case_dirblank,m/n/p= 0, etc. The result was a misleading runtime error:The fix removes the outer
#ifdef MFC_MPIguard so thats_mpi_initialize(),s_assign_default_values_to_user_inputs(),s_read_input_file(), ands_check_input_file()are always called — exactly aspre_processandsimulationalready do. The MPI-specific functions (s_mpi_bcast_user_inputs,s_mpi_decompose_computational_domain, etc.) already guard their own bodies with#ifdef MFC_MPIinternally, so they become no-ops in non-MPI builds.Test plan
--no-mpi) ofpost_processnow readspost_process.inpand processes output correctlyparallel_io = Twith--no-mpiis still caught by the existing@:PROHIBITin the Fortran checker (pre-existing guard)🤖 Generated with Claude Code