Repository navigation
Restore missing input checks and reject unsupported configurations - #2023
Draft
sbryngelson wants to merge 4 commits into
Draft
sbryngelson wants to merge 4 commits into
sbryngelson wants to merge 4 commits into
Conversation
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). (cherry picked from commit 8367254)
MFlowCode#942 (febf131) replaced the hard-coded geometries 7, 13 and 15 with hcid and, in the same edit, dropped the print-only branches for geometries 16-19 from s_check_patches. Those geometries (1D bubble pulse, spiral, 2D and 3D varcircle) are still dispatched in s_apply_icpp_patches and documented in case.md, but since then they fall through to "geometry must be between 1 and 21" and cannot be used. Give each a check routine that enforces its dimensionality (the dispatch silently skips a mismatched one) and the parameters case.md lists. case.md called the varcircle thickness `thickness`; the code reads `epsilon`, so name that instead. Also fix the swapped circle messages: n == 0 now reports "n must be greater than zero" and p > 0 "p must be zero". Verified: a 1D geometry-16 and a 2D geometry-18 case now pass pre_process (master rejects them), and geometry 16 in 2D is rejected with "1D bubble pulse patch 2: n must be zero". (cherry picked from commit 877d0b0)
The loop ran to num_patches_max (10, the ICPP patch limit) although patch_ib holds num_ib_patches_max_namelist entries, so IB patches past the tenth were never validated and inactive slots past it were never checked for stray settings. Loop over all patch_ib slots; unused slots keep their defaults, so the inactive-patch checks do not fire. Also set iStr before branching so the inactive-patch message names the right patch instead of a stale index. Verified with an 11-circle case whose 11th IB has no radius: pre_process now aborts with "in circle IB patch 11" (master accepted it), the same case with a radius passes, and the 61 IBM tests pass. (cherry picked from commit 1c0423e)
post_process's sim_data writers are hard-wired to one configuration: s_write_energy_data_file multiplies by dz (unallocated in 1D/2D), reads adv(2) and alpha_rho(2) and the volume fractions at eqn_idx%E + 1 and eqn_idx%E + 2, and s_write_intf_data_file thresholds q_prim_vf(eqn_idx%E + 2). Any other case read unallocated memory or the wrong variables. Reject sim_data unless p > 0, num_fluids = 2 and model_eqns is 2 or 3, and cover the check in test_case_validator.py. (cherry picked from commit fa17c3a)
This was referenced Oct 10, 2026
Lines of Code
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MUSCL could silently ignore characteristic boundary conditions, valid patch geometries 16–19 were rejected, IB patches beyond slot 10 escaped validation, and unsupported sim_data configurations could read invalid data. This draft combines the four existing input-check fixes and their documentation, examples, and tests.
Consolidation
Each source change is retained as a separate commit, with its original author, message, and a
cherry-pick -xreference. The complete original PR descriptions are reproduced below, including their verification records and limitations. Original review discussions remain available through the source links. Those original verification claims are historical records, not fresh runs on this combined branch.The #1988/#2008 append conflict in
toolchain/mfc/test_case_validator.pywas resolved by retaining both test classes unchanged. No added or deleted source lines from either fix were changed.Verification of this combined branch
git diff --check origin/master...HEADpasses../mfc.sh precheckpasses all seven gates: formatting, spelling, toolchain lint/tests, source lint, documentation references, parameter documentation, and example case validation.This consolidation was performed with OpenAI Codex.
Contribution Policy
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Original PR documentation
#1988: Reject characteristic BCs with MUSCL reconstruction
Source: #1988
Original head:
8367254f08bd029e8534ee3e4d23c6776659c3a5Complete original PR description
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).
#2006: Restore input checks for patch geometries 16-19
Source: #2006
Original head:
877d0b00650b064b70e94c0ff9b3c2ba2466d4f5Complete original PR description
#942 (febf131) replaced the hard-coded geometries 7, 13 and 15 with
hcid and, in the same edit, dropped the print-only branches for
geometries 16-19 from s_check_patches. Those geometries (1D bubble
pulse, spiral, 2D and 3D varcircle) are still dispatched in
s_apply_icpp_patches and documented in case.md, but since then they
fall through to "geometry must be between 1 and 21" and cannot be used.
Give each a check routine that enforces its dimensionality (the
dispatch silently skips a mismatched one) and the parameters case.md
lists. case.md called the varcircle thickness
thickness; the codereads
epsilon, so name that instead.Also fix the swapped circle messages: n == 0 now reports "n must be
greater than zero" and p > 0 "p must be zero".
Verified: a 1D geometry-16 and a 2D geometry-18 case now pass
pre_process (master rejects them), and geometry 16 in 2D is rejected
with "1D bubble pulse patch 2: n must be zero".
#2007: Check every IB patch slot in s_check_ib_patches
Source: #2007
Original head:
1c0423e175a5ddf4fed5b301cc12b64792d9df96Complete original PR description
The loop ran to num_patches_max (10, the ICPP patch limit) although
patch_ib holds num_ib_patches_max_namelist entries, so IB patches past
the tenth were never validated and inactive slots past it were never
checked for stray settings. Loop over all patch_ib slots; unused slots
keep their defaults, so the inactive-patch checks do not fire.
Also set iStr before branching so the inactive-patch message names the
right patch instead of a stale index.
Verified with an 11-circle case whose 11th IB has no radius: pre_process
now aborts with "in circle IB patch 11" (master accepted it), the same
case with a radius passes, and the 61 IBM tests pass.
#2008: Restrict sim_data to 3D two-fluid 5/6-equation cases
Source: #2008
Original head:
fa17c3a3f10a5fff30dd9d47b96a0ae6b3132c5eComplete original PR description
post_process's sim_data writers are hard-wired to one configuration:
s_write_energy_data_file multiplies by dz (unallocated in 1D/2D), reads
adv(2) and alpha_rho(2) and the volume fractions at eqn_idx%E + 1 and
eqn_idx%E + 2, and s_write_intf_data_file thresholds
q_prim_vf(eqn_idx%E + 2). Any other case read unallocated memory or the
wrong variables.
Reject sim_data unless p > 0, num_fluids = 2 and model_eqns is 2 or 3,
and cover the check in test_case_validator.py.