Skip to content

Remove duplicate check_chemistry() calls from stage validators#1627

Open
rayair250-droid wants to merge 1 commit into
MFlowCode:masterfrom
rayair250-droid:fix/dedupe-check-chemistry
Open

Remove duplicate check_chemistry() calls from stage validators#1627
rayair250-droid wants to merge 1 commit into
MFlowCode:masterfrom
rayair250-droid:fix/dedupe-check-chemistry

Conversation

@rayair250-droid

Copy link
Copy Markdown

Addresses part (b) of #1509.

check_chemistry() ran twice per validation: once in validate_common() (which every stage calls) and again at the end of validate_simulation, validate_pre_process, and validate_post_process. Because prohibit() appends to self.errors whenever its condition holds, a single violated chemistry constraint was reported twice. This removes the three redundant per-stage calls; validate_common() already covers all stages, so coverage is unchanged and behavior is identical except that duplicate error-list entries no longer appear.

Smallest-correct-change: 3 deletions, no additions.

On part (a) of the issue (left out on purpose)

I checked the fallback-constant drift against current master and it has partly moved since the issue was filed, so I did not apply the values from the issue:

  • The num_ib_patches_max fallback site the issue cited (case_validator.py:585) no longer exists.
  • In src/common/m_constants.fpp, num_ib_patches_max is now 2050000 (the issue said 50000).
  • The only remaining fallback mismatch is num_patches_max (fallback 1000 vs real 10); num_bc_patches_max already matches.

Since (a) is a never-normally-hit fallback path (hygiene only, as the issue notes) and its premise has drifted, I kept this PR to the clear (b) fix. Happy to do the num_patches_max fallback in a small follow-up if you want it.

check_chemistry() ran twice per validation: once in validate_common() (called by every stage) and again in validate_simulation/pre_process/post_process. A violated chemistry constraint was therefore reported twice. Remove the three redundant per-stage calls; validate_common() covers all stages. Part (b) of MFlowCode#1509.
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.43%. Comparing base (66254db) to head (2d1a707).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1627   +/-   ##
=======================================
  Coverage   60.43%   60.43%           
=======================================
  Files          83       83           
  Lines       19868    19868           
  Branches     2956     2956           
=======================================
  Hits        12007    12007           
  Misses       5860     5860           
  Partials     2001     2001           

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

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