mlcommons / mlcommons/storage

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

Open
#842 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

mlpstorage
Dominant language
Python
Stars
203
Forks
67
Avg merge
20m
Merged PRs (30d)
8

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in checkpointing_checks.py at the four named functions and in training_checks.py at closed_submission_parameters and open_submission_parameters; compare their verification checks with the fixed 4.6.1 and 4.3.5 gates. Run the existing tests, then validate a clone of the current submissions tree and record new error lines by rule and organization. Done means the six checks handle uppercase metadata, regression coverage prevents fixture drift, and the before/after delta is published.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.