Skip to content

Six submission-checker rules never fire on real trees: verification compared lowercase-only against uppercase metadata #842

Description

@FileSystemGuy

Summary

mlpstorage writes the division into run metadata as verification: "CLOSED" / "OPEN" (uppercase). Six submission-checker rule functions compare that field against lowercase literals without folding case, so their division-gated bodies never (or always wrongly) match on a real tree. Every one of them is currently dead or partially dead in production, while passing its unit tests — the test fixtures write lowercase.

Found while verifying the #841 fix: a validate sweep over a clone of the v3.0 submissions tree returned counts byte-identical to the pre-fix baseline, because the new CLOSED gate — copied from the surrounding checks' idiom — skipped every real run. 4.6.1 checkpointClosedMpiProcesses had the same bug and has therefore never fired on a real tree; it was fixed inside the #841 change (ab0edd8) because #841's detection depends on it.

Affected checks (still unfixed)

Rule Function Site Effect on a real tree
4.6.2 checkpointClosedAcceleratorsPerHost closed_accelerators_per_host checkpointing_checks.py:295-297 never fires
4.6.3 checkpointClosedCheckpointParameters closed_checkpoint_parameters checkpointing_checks.py:457-458 never fires
4.6.4 checkpointOpenSubmissionScaling open_mpi_processes checkpointing_checks.py:624-625 never fires (!= "open" is always true)
4.7.1 checkpointCacheFlushValidation checkpoint_invocation_structure checkpointing_checks.py:847 CLOSED-gated branch never taken
3.6.2 trainingClosedSubmissionParameters closed_submission_parameters training_checks.py:879-881 never fires
3.6.3 trainingOpenSubmissionParameters open_submission_parameters training_checks.py:948-950 never fires

Why not fixed alongside #841

Enabling six dormant checks changes real-tree validate output in ways nobody has measured — 4.6.3 alone (CLOSED parameter whitelisting) could produce a large new error family across the v3.0 tree. That needs its own change with a clone-gated before/after sweep quantifying every new error line and attributing it, per the established regeneration procedure, so review staff can disposition the findings deliberately rather than discovering them in a routine run.

Suggested fix

  1. Fold case at each comparison (metadata.get("verification", ...).lower()), matching what 4.6.1 and the 4.3.5 gate now do.
  2. Add a shared helper or a test asserting fixtures and production agree on the field's canonical form, so the fixture/reality divergence cannot silently return.
  3. Before merging, run validate over a clone of the current submissions tree and publish the delta (new error lines per rule, per organization) in the PR.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions