Skip to content

case_validator: check_chemistry() runs twice + stale fallback constants disagree with m_constants.fpp #1509

@sbryngelson

Description

@sbryngelson

Two independent validator-hygiene problems. Part (b) is a real (cosmetic) bug; part (a) is hygiene only.

(b) check_chemistry() is invoked twice per validation — confirmed. It is called in validate_common (case_validator.py:2069) and again in each stage method: validate_simulation (2094), validate_pre_process (2104), validate_post_process (2132). Since prohibit() appends to self.errors whenever its condition is true, a single violated chemistry constraint is reported twice. Fix: remove the redundant self.check_chemistry() from the three stage methods (validate_common already covers all stages), or keep it per-stage — either way it should run once.

Correction to the original report: the claim that check_finite_difference/check_time_stepping are also double-called is wrong — both are defined once and called once per stage that needs them (not in validate_common). Only check_chemistry is duplicated.

(a) Fallback constants disagree with source of truth — hygiene only, NOT a live validation gap. Three sites read compile-time limits with a hardcoded fallback:

# case_validator.py:585
num_ib_patches_max = get_fortran_constants().get("num_ib_patches_max", 100000)   # real value: 50000
# case_validator.py:1218
num_patches_max    = get_fortran_constants().get("num_patches_max", 1000)         # real value: 10
# case_validator.py:1481
num_bc_patches_max = get_fortran_constants().get("num_bc_patches_max", 10)        # real value: 10 (matches)

The real values in src/common/m_constants.fpp:26-28 are num_patches_max = 10, num_ib_patches_max = 50000, num_bc_patches_max = 10. So two fallbacks are wrong. However get_fortran_constants() parses these directly from m_constants.fpp and normally succeeds — the fallback is only used if that parse fails, so the wrong fallbacks do not loosen real validation in practice. It's a "second source of truth that has drifted" issue. Fix: make the fallbacks match m_constants.fpp (or drop the fallback and treat a parse miss as an error, since these constants are required).

Behavior preservation: (b) only removes duplicate error-list entries; (a) only changes the never-normally-hit fallback path. Toolchain-only.


Filed from a repo-wide code-cleanliness review; verified against master @ 40dde5e.

Code references

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working or doesn't seem rightgood first issueGood for newcomers

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions