Skip to content

Restore missing input checks and reject unsupported configurations - #2023

Draft
sbryngelson wants to merge 4 commits into
MFlowCode:masterfrom
sbryngelson:consolidate/validation-fixes
Draft

sbryngelson wants to merge 4 commits into
MFlowCode:masterfrom
sbryngelson:consolidate/validation-fixes

Conversation

@sbryngelson

@sbryngelson sbryngelson commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

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 -x reference. 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.py was resolved by retaining both test classes unchanged. No added or deleted source lines from either fix were changed.

Verification of this combined branch

  • Every source commit retains exactly the same added and deleted lines, verified against the original commit.
  • git diff --check origin/master...HEAD passes.
  • ./mfc.sh precheck passes all seven gates: formatting, spelling, toolchain lint/tests, source lint, documentation references, parameter documentation, and example case validation.
  • Full solver regressions and multi-rank numerical runs were not repeated for this consolidation; their original records are preserved below, and CI should validate the new head.

This consolidation was performed with OpenAI Codex.

Contribution Policy

All contributions are expected to demonstrate:

  • A clear understanding of the codebase
  • Alignment with product direction
  • Thoughtful reasoning behind changes
  • Evidence of real-world usage or hands-on experience with the problem

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: 8367254f08bd029e8534ee3e4d23c6776659c3a5

Complete 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: 877d0b00650b064b70e94c0ff9b3c2ba2466d4f5

Complete 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 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".

#2007: Check every IB patch slot in s_check_ib_patches

Source: #2007

Original head: 1c0423e175a5ddf4fed5b301cc12b64792d9df96

Complete 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: fa17c3a3f10a5fff30dd9d47b96a0ae6b3132c5e

Complete 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.

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)
@github-actions

Copy link
Copy Markdown

Lines of Code

File Lines Diff
src/pre_process/m_check_patches.fpp 431 +49
Directory Lines Diff
pre_process 5083 +49
total 47475 +49

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.

1 participant