Fix MUSCL THINC right-state using already-overwritten left-state values#1181
Fix MUSCL THINC right-state using already-overwritten left-state values#1181sbryngelson wants to merge 1 commit 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 · |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Pull request overview
Fixes MUSCL-THINC right-state reconstruction by preserving pre-overwrite density ratios from the left state and reusing them for both left and right reconstructions.
Changes:
- Save pre-reconstruction density ratios (
rho_b,rho_e) before THINC overwritesvL_rs_vf_*values. - Update left/right reconstructions to use the saved ratios instead of dividing by potentially overwritten left-state fields.
The right reconstruction divides by left-state values that were already overwritten during left reconstruction. Save original density ratios (contxb/advxb and contxe/(1-advxb)) before left reconstruction and reuse them for both left and right states. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
afa042e to
66a6a8c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1181 +/- ##
=======================================
Coverage 44.05% 44.05%
=======================================
Files 70 70
Lines 20498 20496 -2
Branches 1990 1990
=======================================
Hits 9030 9030
+ Misses 10329 10327 -2
Partials 1139 1139 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Severity: HIGH — right-state reconstruction uses overwritten left-state values.
File:
src/simulation/m_muscl.fpp, lines 285-301In the MUSCL-THINC interface reconstruction, the left-state values (
vL_rs_vfforcontxb,contxe, andadvxb) are overwritten during left reconstruction. The right-state reconstruction then divides by these already-overwritten values instead of the original cell-center values. While the math accidentally cancels (left THINC value / left THINC value = original ratio), this relies on exact cancellation and is fragile.Before
After
Why this went undetected
The algebraic cancellation makes this produce correct results in most cases. It would only fail if the THINC clamping pushed
aTHINCto exactly 0 or 1, causing 0/0.Test plan
🤖 Generated with Claude Code
Fixes #1202