Skip to content

toolchain: fix KeyError('num_fluids') in case-optimization fpp generation#1641

Merged
sbryngelson merged 1 commit into
MFlowCode:masterfrom
sbryngelson:fix-caseopt-num_fluids-keyerror
Jul 12, 2026
Merged

toolchain: fix KeyError('num_fluids') in case-optimization fpp generation#1641
sbryngelson merged 1 commit into
MFlowCode:masterfrom
sbryngelson:fix-caseopt-num_fluids-keyerror

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Summary

Case.__get_sim_fpp (toolchain/mfc/case.py) reads self.params["num_fluids"] by direct subscript when emitting the case-optimization Fypp header, while every other parameter in that same block uses self.params.get(key, default).

The packer constructs a dummy case with empty params to enumerate configured targets for the golden metadata file:

# toolchain/mfc/packer/pack.py
case = input.load(None, {}, {})
for target in get_configured_targets(case):
    ...

Under --case-optimization, that empty-param case flows through get_fpp__get_sim_fpp, and the bare subscript raises KeyError: 'num_fluids'.

Impact

./mfc.sh test --case-optimization fails for every golden case with Error: 'num_fluids' — even though the case-optimized simulation itself builds and runs correctly. The failure is in the post-run golden-packing/metadata step, not the compiled binary (the run completes with exit 0 and correct output before the packer is invoked).

CI's case-optimization job runs ./mfc.sh run --case-optimization + a custom output check (.github/scripts/run_case_optimization.sh), not the golden harness, so it never hits this path — which is why it went unnoticed.

Fix

One line, matching the .get(..., default) pattern of every sibling in the same block (nb, weno_order, muscl_order, …):

-#:set num_fluids            = {int(self.params["num_fluids"])}
+#:set num_fluids            = {int(self.params.get("num_fluids", 1))}

For the dummy empty case the default is only used to compute a throwaway staging slug (which won't match any real build and is skipped); for real cases num_fluids is present, so behavior is unchanged.

Testing

  • Reproduced: ./mfc.sh test --only <AMR golden> --case-optimizationKeyError: 'num_fluids' before the change; passes after.
  • Verified on GPU (V100) with both OpenACC and OpenMP offload: two AMR multi-level goldens pass under --case-optimization (2/2 on each backend) after the fix.

…tion

case.py's __get_sim_fpp reads self.params["num_fluids"] by bracket, while
every other parameter in that case-optimization block uses .get(key, default).
The packer builds a dummy case with empty params (pack.py: input.load(None, {}, {}))
to enumerate configured targets for golden metadata; under --case-optimization
that flows into get_fpp and the bracket access raises KeyError('num_fluids').
This breaks './mfc.sh test --case-optimization' (golden harness) even though the
simulation itself runs correctly. Use .get("num_fluids", 1) to match the
surrounding lines.
Copilot AI review requested due to automatic review settings July 11, 2026 21:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a toolchain crash during case-optimization Fypp header generation when a “dummy” case is created with empty parameters (used by the golden packer to enumerate targets). This prevents KeyError('num_fluids') in Case.__get_sim_fpp while keeping behavior unchanged for real cases where num_fluids is set.

Changes:

  • Replace direct self.params["num_fluids"] access with self.params.get("num_fluids", 1) to avoid KeyError for empty-param dummy cases.

@codecov

codecov Bot commented Jul 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.95%. Comparing base (8285ba0) to head (46d8b62).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1641   +/-   ##
=======================================
  Coverage   60.95%   60.95%           
=======================================
  Files          83       83           
  Lines       20003    20003           
  Branches     2983     2983           
=======================================
  Hits        12193    12193           
  Misses       5782     5782           
  Partials     2028     2028           

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

@sbryngelson

Copy link
Copy Markdown
Member Author

apropos of little

The fix itself → closing #1641 does have a (small, real) consequence: it abandons a correct 1-line bug fix and leaves ./mfc.sh test --case-optimization broken (KeyError: 'num_fluids') on master.

Recall what #1641 actually fixes: case.py:394 reads self.params["num_fluids"] by bracket while every sibling uses .get(...). The packer builds an empty-params dummy case, so under --case-optimization that path KeyErrors — I hit it live earlier this session. It's a genuine latent bug, present on master right now.

The nuance:

  • It's niche. CI's case-opt (and the normal workflow) use ./mfc.sh run --case-optimization + a custom validator, which never touches the golden-packer path. So CI doesn't exercise the bug — which is exactly why it went unnoticed.
  • But it's a real dev-facing break. Anyone who runs the golden test harness with --case-optimization (a legitimate way to check case-opt against goldens) hits the KeyError immediately.

@sbryngelson sbryngelson merged commit 39a39b8 into MFlowCode:master Jul 12, 2026
232 of 263 checks passed
@sbryngelson sbryngelson deleted the fix-caseopt-num_fluids-keyerror branch July 12, 2026 15:15
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