Fix MHD hyperbolic cleaning using wrong sign for left fast magnetosonic speed#1180
Fix MHD hyperbolic cleaning using wrong sign for left fast magnetosonic speed#1180sbryngelson wants to merge 2 commits intoMFlowCode:masterfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughThis PR fixes a sign error in the MHD Riemann solver's hyperbolic divergence cleaning wave-speed bound calculation. The left fast magnetosonic speed bound changes from a minimum operation with negated speed to a maximum operation with positive speed. Test metadata files are updated to reflect a new build environment and hardware platform. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Pull request overview
Fixes the sign handling for left-state fast magnetosonic speed enforcement when hyperbolic cleaning is enabled, making it consistent with the right-state treatment.
Changes:
- Replace
min(c_fast%L, -hyper_cleaning_speed)withmax(c_fast%L, hyper_cleaning_speed)under hyperbolic cleaning. - Keep right-state enforcement as
max(c_fast%R, hyper_cleaning_speed)for symmetry.
…ic speed min(c_fast%L, -hyper_cleaning_speed) forces c_fast%L negative, but fast magnetosonic speeds should be positive. Changed to max() to match the right-state treatment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5049adc to
d51aaa7
Compare
Update golden files for hyper_cleaning and mhd_rotor tests to reflect the corrected min/max sign for the left-state fast magnetosonic speed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
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 `@tests/F057F8E6/golden-metadata.txt`:
- Line 6: The CI is running test F057F8E6 with MPI=ON by default while the
golden metadata shows mpi=No; update the CI matrix entry for the test id
F057F8E6 to force mpi=no-mpi (set the matrix key "mpi" to the no-mpi variant for
that job), or alternatively add F057F8E6 to the test exclusions when mpi is ON,
or regenerate the golden for both mpi=Yes and mpi=No so CI and the golden
configs match. Ensure you modify the workflow matrix "mpi" setting or the test
include/exclude entries that reference F057F8E6 accordingly.
---
Duplicate comments:
In `@tests/B4DC99F9/golden-metadata.txt`:
- Line 7: The golden metadata for the mhd_rotor test records a dirty git state
("(dirty)"), indicating uncommitted changes at generation; reproduce a
clean-state golden by checking out a clean commit/branch, committing or stashing
any local changes, regenerating the golden output, and updating the metadata so
the Git line for mhd_rotor no longer contains "(dirty)"; reference the golden
identifier "mhd_rotor" and the related test metadata file and follow the same
verification/cleanup steps used for the previously reported F057F8E6 occurrence
to ensure parity.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1180 +/- ##
=======================================
Coverage 44.05% 44.05%
=======================================
Files 70 70
Lines 20498 20498
Branches 1990 1990
=======================================
Hits 9030 9030
Misses 10329 10329
Partials 1139 1139 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@sbryngelson Thanks for fixing this. Hyperbolic cleaning expands the HLL fan to include the new eigenvalues. In less extreme cases, the wrong formulas had little impact, so the code ran fine with my examples. I've added a few more changes, and now it matches theory and is fully robust. I don't seem to have write access to this branch, so I'm opening another PR #1235. |
|
Closing in favor of #1235, which incorporates this fix and adds additional MHD wave speed improvements. |
User description
Summary
Severity: HIGH — wrong wave speed estimate corrupts MHD flux computation.
File:
src/simulation/m_riemann_solvers.fpp, lines 655-657The hyperbolic divergence cleaning should ensure that the fast magnetosonic speed is at least the cleaning speed in both directions. The right state correctly uses
max(c_fast%R, hyper_cleaning_speed), but the left state usesmin(c_fast%L, -hyper_cleaning_speed), which forces it negative.Before
After
Why this went undetected
MHD with hyperbolic cleaning is a specialized feature. The asymmetric wave speed produces subtly wrong fluxes that may not cause immediate crashes.
Test plan
🤖 Generated with Claude Code
Fixes #1201
CodeAnt-AI Description
Fix wrong sign for left fast magnetosonic speed in MHD hyperbolic cleaning
What Changed
Impact
✅ Correct MHD fluxes when hyperbolic cleaning is enabled✅ Fewer corrupted or asymmetric wave solutions in affected simulations✅ Updated regression goldens to match corrected behavior💡 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.
Summary by CodeRabbit
Bug Fixes
Tests