Skip to content

Reject characteristic BCs with MUSCL reconstruction - #2129

Open
sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave1/muscl-cbc-validator
Open

sbryngelson wants to merge 1 commit into
MFlowCode:masterfrom
sbryngelson:wave1/muscl-cbc-validator

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

What

s_cbc computes the characteristic boundary fluxes only when
recon_type == WENO. With MUSCL the routine copies the fluxes in and out
unchanged, so a characteristic BC (-5 to -12) silently behaves like
extrapolation (-3): a MUSCL run with bc = -5 or -6 is bit-identical to
the same run with bc = -3.

Prohibit the combination in check_muscl_simulation, sharing the
characteristic-BC scan with the existing IGR check, and add unit tests.

2D_shockdroplet_muscl and 3D_shockdroplet_muscl used bc_x = -6 with
MUSCL; switch them to -3, which they were already getting. Their
outputs are unchanged (bit-identical p_all and D on the 2D test case;
tests BE796E10 and 23573861 pass against the existing goldens).

Verification

Testing. Built with GPU offload (OpenMP, AMD) and ran the full test suite with this fix and seven other independent fixes applied together: 729 tests in 4 shards, 0 failures (two shards on MI300A, two on MI250X).

Prepared with Claude Code (AI-assisted); I reviewed the change and ran the tests.

s_cbc computes the characteristic boundary fluxes only when
recon_type == WENO. With MUSCL the routine copies the fluxes in and out
unchanged, so a characteristic BC (-5 to -12) silently behaves like
extrapolation (-3): a MUSCL run with bc = -5 or -6 is bit-identical to
the same run with bc = -3.

Prohibit the combination in check_muscl_simulation, sharing the
characteristic-BC scan with the existing IGR check, and add unit tests.

2D_shockdroplet_muscl and 3D_shockdroplet_muscl used bc_x = -6 with
MUSCL; switch them to -3, which they were already getting. Their
outputs are unchanged (bit-identical p_all and D on the 2D test case;
tests BE796E10 and 23573861 pass against the existing goldens).
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Claude Code Review

Head SHA: b85b0ad

Files changed:

  • 4
  • examples/2D_shockdroplet_muscl/case.py
  • examples/3D_shockdroplet_muscl/case.py
  • toolchain/mfc/case_validator.py
  • toolchain/mfc/test_case_validator.py

Findings:
No issues found. Checked in step 4: toolchain/mfc/case_validator.py (the new check sits after the recon_type != 2 early return in check_muscl_simulation, and the IGR refactor keeps the same -12..-5 range), toolchain/mfc/test_case_validator.py, and both shockdroplet example cases.

@sbryngelson
sbryngelson marked this pull request as ready for review November 3, 2026 14:17
@sbryngelson
sbryngelson requested a review from a team as a code owner November 3, 2026 14:17
@sbryngelson
sbryngelson requested review from danieljvickers and removed request for a team November 3, 2026 14:17
@codecov

codecov Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.90%. Comparing base (a03848a) to head (b85b0ad).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2129   +/-   ##
=======================================
  Coverage   61.90%   61.90%           
=======================================
  Files          86       86           
  Lines       22948    22948           
  Branches     3386     3386           
=======================================
  Hits        14205    14205           
  Misses       6229     6229           
  Partials     2514     2514           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants