Fix 6 low-risk pre-process bugs (batch)#1241
Conversation
codeant-ai
bot
added
the
size:S
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Nitpicks 🔍
|
There was a problem hiding this comment.
Pull request overview
This PR batches 6 bug fixes in pre-process code paths that address real numerical issues in initial condition generation. The fixes correct type declarations, array bounds, duplicate operations, statement ordering, loop coverage, and MPI broadcast placement.
Changes:
- Fixed
moncon_cutoffdeclared asintegerinstead ofreal(wp), which silently truncated1e-8to0and disabled MUSCL-THINC monotonicity limiting - Corrected grid stretching array bounds for
y_ccandz_ccusing wrong dimension (minstead ofnandp) - Removed duplicate R3bar accumulation line that doubled bubble perturbation magnitude in QBMM initial conditions
- Reordered perturbation statements so y-velocity uses original x-velocity before x-velocity is modified
- Extended hyper-cleaning psi initialization to cover all z-planes in 3D (was only initializing k=0 plane)
- Moved IB patch velocity/angular_vel/angles broadcasts from wrong loop to correct loop
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/common/m_constants.fpp | Changed moncon_cutoff from integer to real(wp) to prevent truncation |
| src/pre_process/m_grid.f90 | Fixed y_cc and z_cc array bounds to use correct dimensions (n and p instead of m) |
| src/pre_process/m_assign_variables.fpp | Removed duplicate R3bar accumulation line in QBMM branch |
| src/pre_process/m_perturbation.fpp | Swapped statement order so y-velocity perturbation uses unmodified x-velocity |
| src/pre_process/m_start_up.fpp | Added outer z-loop for 3D psi initialization and replaced double-precision literal |
| src/pre_process/m_mpi_proxy.fpp | Moved patch_ib vel/angular_vel/angles broadcasts to num_patches_max loop |
codeant-ai
bot
added
size:S
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1241 +/- ##
==========================================
- Coverage 44.95% 44.95% -0.01%
==========================================
Files 70 70
Lines 20504 20509 +5
Branches 1946 1949 +3
==========================================
+ Hits 9217 9219 +2
- Misses 10166 10169 +3
Partials 1121 1121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codeant-ai
bot
added
size:S
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pre_process/m_start_up.fpp`:
- Around line 790-797: Add a validator in the CaseValidator
(toolchain/mfc/case_validator.py) to forbid enabling hyper_cleaning in 2D: call
self.prohibit(hyper_cleaning and p is not None and p == 0, "Hyperbolic cleaning
is not supported for 2D simulations") in the validation routine that checks
dimensionality (the same place that already forbids 1D via n==0); this prevents
m_start_up.fpp from accessing unallocated z_cc when p==0 by rejecting
hyper_cleaning for 2D cases.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/common/m_constants.fppsrc/pre_process/m_assign_variables.fppsrc/pre_process/m_grid.f90src/pre_process/m_mpi_proxy.fppsrc/pre_process/m_perturbation.fppsrc/pre_process/m_start_up.fpp
💤 Files with no reviewable changes (1)
- src/pre_process/m_assign_variables.fpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/common/m_constants.fpp
- src/pre_process/m_mpi_proxy.fpp
sbryngelson
force-pushed
the
fix/low-risk-bugfixes-batch
branch
from
285998d to
068c79c
Compare
codeant-ai
bot
added
size:S
sbryngelson
force-pushed
the
fix/low-risk-bugfixes-batch
branch
2 times, most recently
from
9276e0b to
96f562c
Compare
codeant-ai
bot
added
size:S
sbryngelson
force-pushed
the
fix/low-risk-bugfixes-batch
branch
from
1dcf0a3 to
22304a1
Compare
The psi field used by hyperbolic cleaning is initialized with a 3D loop that accesses z_cc, which is out-of-bounds when p == 0. The 1D prohibition already existed; extend it to 2D. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In 1D simulations n==0 and p==0, so both the 1D and 2D prohibition checks were firing simultaneously. Gate the 2D check on n > 0 so it only triggers for configurations that are actually 2D. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GLM divergence cleaning is valid in 2D. The z_cc access was already guarded with `if (p > 0)` in 22304a1. The prohibition broke the existing 2D hyper_cleaning, mhd_rotor, and orszag_tang tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The @:ASSERT call added in d614938 requires the macros.fpp include, which was missing, causing Fypp preprocessing to fail in the documentation build. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sbryngelson
force-pushed
the
fix/low-risk-bugfixes-batch
branch
from
df91890 to
732146c
Compare
The linter will intentionally fail until PRs MFlowCode#1187 (WP_MOK), MFlowCode#1241 (R3bar duplicate), and this PR's dq_prim_dy fix (now in MFlowCode#1187) merge first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Summary by QodoFix six low-risk pre-process bugs in IC generation
WalkthroughsDescription• Fixes moncon_cutoff type from integer to real, restoring THINC monotonicity limiting • Corrects stretched-grid cell-center bounds for y and z dimensions using proper array sizes • Removes duplicate R3bar accumulation that doubled QBMM bubble perturbation magnitude • Fixes y-velocity perturbation order to use unmodified x-velocity values • Extends hyper-cleaning psi initialization across all 3D planes with proper 2D guards • Moves immersed-boundary variable broadcasts to correct loop scope for proper MPI propagation Diagramflowchart LR
A["Pre-process bugs"] --> B["moncon_cutoff type fix"]
A --> C["Grid stretching bounds"]
A --> D["QBMM perturbation"]
A --> E["Velocity perturbation order"]
A --> F["Hyper-cleaning 3D init"]
A --> G["IB broadcast scope"]
B --> H["THINC limiting restored"]
C --> I["Correct coordinates"]
D --> J["Correct amplitude"]
E --> K["Correct values"]
F --> L["Full 3D coverage"]
G --> M["Reliable MPI sync"]
File Changes1. src/common/m_constants.fpp
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
Claude Code ReviewHead SHA: Summary
FindingsNo blocking issuesAll six fixes look correct. Detailed per-fix notes:
Minor / Non-blocking
VerdictAll fixes are correct, well-scoped, and adhere to MFC conventions. Approved. |
Claude Code ReviewHead SHA: Files changed: 6
Summary
Findings1.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request contains multiple corrections and enhancements across preprocessing modules. Changes include: converting a constant type from integer to real, correcting array indexing in grid cell-center coordinate calculations, expanding initial condition setup from two dimensions to three dimensions with corresponding loop restructuring, removing a duplicate accumulation line in a calculation routine, adjusting MPI broadcast sizes from fixed to dynamically determined values, and correcting documentation in parameter descriptions and comments. 🚥 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 |
Claude Code ReviewHead SHA: f6ab8ce Summary
Findings1. 2. 3. 4. 5. 6. Minor observations (non-blocking)
|
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoFix six low-risk pre-process bugs in IC generation and MPI setup
WalkthroughsDescription• Fixes moncon_cutoff type from integer to real, restoring THINC monotonicity limiting • Corrects stretched-grid cell-center bounds for y and z dimensions using proper array sizes • Removes duplicate R3bar accumulation line that doubled QBMM bubble perturbation magnitude • Fixes y-velocity perturbation computation order to use unmodified x-velocity • Extends hyper-cleaning psi initialization across all 3D planes with proper 2D guards • Moves immersed-boundary variable broadcasts to correct loop scope and fixes numeric literals Diagramflowchart LR
A["Pre-process Code"] --> B["Bug Fixes"]
B --> C["THINC Limiting"]
B --> D["Grid Stretching"]
B --> E["QBMM Perturbation"]
B --> F["Velocity Order"]
B --> G["Hyper-cleaning Init"]
B --> H["IB Broadcasts"]
C --> I["Correct IC"]
D --> I
E --> I
F --> I
G --> I
H --> J["Correct MPI Setup"]
File Changes1. src/common/m_constants.fpp
|
Code Review by Qodo
1. Missing tests for common change
|
Claude Code ReviewHead SHA: f6ab8ce Files changed: 6
Summary
Findings
OverallThe batch is ready to merge. All fixes are straightforward, well-motivated, and consistent with MFC conventions. No issues with precision discipline, GPU macros, or compiler portability. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| real(wp), parameter :: dflt_ic_eps = 1e-4_wp !< Ensure compression is only applied to surface cells in THINC | ||
| real(wp), parameter :: dflt_ic_beta = 1.6_wp !< Sharpness parameter's default value used in THINC | ||
| integer, parameter :: moncon_cutoff = 1e-8_wp !< Monotonicity constraint's limiter to prevent extremas in THINC | ||
| real(wp), parameter :: moncon_cutoff = 1e-8_wp !< Monotonicity constraint's limiter to prevent extremas in THINC |
There was a problem hiding this comment.
1. Missing tests for common change 📘 Rule violation ⛯ Reliability
This PR modifies shared code under src/common/ but provides no evidence that pre_process, simulation, and post_process were all tested. This increases regression risk across multiple executables that depend on the common constants module.
Agent Prompt
## Issue description
This PR changes `src/common/` but does not provide evidence that all three executables (pre_process, simulation, post_process) were tested, which is required for shared-code modifications.
## Issue Context
The change touches shared constants used across targets, so regressions can impact multiple executables.
## Fix Focus Areas
- src/common/m_constants.fpp[47-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoFix six low-risk pre-process bugs affecting initial conditions
WalkthroughsDescription• Fixes moncon_cutoff type from integer to real, restoring THINC monotonicity limiting • Corrects stretched-grid cell-center calculations for y and z dimensions using wrong bounds • Removes duplicate R3bar accumulation line that doubled QBMM bubble perturbation magnitude • Fixes y-velocity perturbation reading already-modified x-velocity instead of original • Extends hyper-cleaning psi initialization to all 3D planes with proper 2D guards • Moves immersed-boundary variable broadcasts to correct loop scope for proper MPI propagation Diagramflowchart LR
A["Pre-process Code"] --> B["Bug Fixes"]
B --> C["moncon_cutoff Type"]
B --> D["Grid Stretching Bounds"]
B --> E["QBMM Perturbation"]
B --> F["Velocity Perturbation Order"]
B --> G["Hyper-cleaning 3D Init"]
B --> H["IB MPI Broadcasts"]
C --> I["Correct THINC Limiting"]
D --> J["Accurate Coordinates"]
E --> K["Correct Amplitudes"]
F --> L["Proper Velocity Values"]
G --> M["Full 3D Coverage"]
H --> N["Reliable MPI Propagation"]
File Changes1. src/common/m_constants.fpp
|
Code Review by Qodo
1. Missing tests for common change
|
Claude Code ReviewHead SHA: f6ab8ce Summary
FindingsNo blocking issues found. A few minor observations:
VerdictAll fixes are correct, focused, and match MFC coding conventions. Approved from a code-review standpoint — the batch is ready to merge pending CI green. |
User description
Summary
Batches 6 low-risk bug fixes in pre-process code paths. These fix real numerical issues in IC generation but don't touch simulation GPU kernels or MPI hot paths.
Included fixes
moncon_cutofftype (Fix moncon_cutoff declared as integer, truncating 1e-8 to 0 #1174): Declared asinteger, silently truncating1e-8to0— disabled MUSCL-THINC monotonicity limitery_cc(0:m)andz_cc(0:m)used x-dimension bound instead of0:n/0:ppsiinitialization only coveredk=0plane in 3D MHD casesvel/angular_vel/anglesbroadcast was inside the wrong loopSupersedes
Closes #1174, closes #1179, closes #1183, closes #1216, closes #1217, closes #1188, fixes #1197, fixes #1202, fixes #1206, fixes #1208, fixes #1221, fixes #1222
Test plan
🤖 Generated with Claude Code
CodeAnt-AI Description
Fix pre-process bugs that caused incorrect initial conditions and MPI broadcasts
What Changed
Impact
✅ Correct THINC interface limiting✅ Accurate stretched-grid coordinates✅ Correct QBMM bubble perturbation amplitudes💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.