Skip to content

Report every problem in one checker run - #110

Merged
arav-agarwal2 merged 1 commit into
mainfrom
feat/checker-reports-everything
Oct 7, 2026
Merged

arav-agarwal2 merged 1 commit into
mainfrom
feat/checker-reports-everything

Conversation

@arav-agarwal2

Copy link
Copy Markdown
Collaborator

The checker should report everything wrong with a submission in one run, so a submitter isn't fixing one round of errors at a time. Today an early mistake often hides the checks it doesn't affect. This PR stops that.

Before and after

Each row is the compliant valid_standardized submission with one mistake introduced:

Mistake Before After
none 231 results, 51 rules 219 results, 51 rules (shared-path passes grouped)
docs/ missing 3 results: everything else was hidden all 51 rules; 2 errors (required-dir, and the shared_docs pointer once)
division: Standardised typo in system_desc.json 75 results, 19 rules: the whole curve was skipped all 51 rules; the 1 real error
one bad field in r16's point.yaml 3 errors, 2 of them false (low-concurrency-coverage, accuracy-coverage) the 1 real error, plus a warning naming what couldn't run for r16
AMD's v0.7-layout submission (39e8dd5d) 2 bare "missing directory" errors both errors name the v0.7 layout, plus a third real error that was hidden: src/Llama-3_1-8B/ has no README.md

Changes

  • Missing directories (SubmissionChecker.run). Only a missing results/ ends the results checks; a missing docs/ is reported and the run continues. src/ is checked once per submission, even when results/ is missing, instead of once per curve.
  • v0.7 hint (SubmissionDir). If pareto/ or documentation/ exist, the missing-directory errors say this looks like the v0.7 layout and point to §8.1.
  • Invalid system_desc.json (_check_model). Its errors are reported and the curve keeps going. C_max falls back to the raw max_supported_concurrency. If that's unreadable too, only the region-dependent rules are skipped, with a region-basis warning saying so. ModelContext.system_desc is now optional, with a raw_c_max fallback; only the two offline rules compare against C_max.
  • A point whose point.yaml is missing or invalid still counts (_load_point_configs, _derive_regions, ModelContext). Its concurrency comes from the raw field when that's a positive integer, else from its r<N> name, so it still feeds C_min, coverage and accuracy coverage. Its result_summary.json and accuracy files are still checked. A point-rules-skipped warning lists the rules that need the parsed config (placement, metric consistency, power), so the submitter knows fixing the file may surface more.
  • shared-path-resolution reports once per distinct value, naming the points, like model-name-valid already does. Otherwise a missing docs/ would be the same error once per point.

A reversed design decision, for review

TestPartialAndAbsentBasis said unparseable points must be left out of C_min because their concurrency isn't trustworthy. Leaving them out moves the floor anyway, which is what produced the false coverage errors above. The fallback value is the submitter's declared concurrency: either the raw field, or the r<N> name that point-dirname-concurrency already holds to it. The two affected tests are updated to the new behavior; their old assertions encoded the false errors.

Tests

The new tests/submission_checker/test_report_completeness.py has 11 tests: each introduces one mistake into a copy of valid_standardized and asserts that the mistake is reported, no rule from the clean run is lost, and no error is invented. Full suite: 1214 passed (non-integration). ruff and mypy are clean.

🤖 Generated with Claude Code

A mistake no longer hides the checks it does not affect, so a submitter
sees everything to fix at once instead of one round at a time.

- A missing docs/ no longer stops the run; only a missing results/ ends
  the results checks. src/ is checked once per submission, whatever else
  is missing, instead of once per curve.
- A v0.7-shaped submission (pareto/, documentation/) is told so, rather
  than seeing two unrelated missing directories.
- An invalid system_desc.json is reported without dropping the curve:
  C_max falls back to the raw max_supported_concurrency, and only rules
  that need it are skipped when that is unreadable too.
- A point whose point.yaml is missing or invalid still counts. Its
  concurrency comes from the raw field or its r<N> name, so it no longer
  shifts C_min and the region boundaries, which used to report covered
  regions as empty. Its result summary and accuracy are still checked,
  and a warning names the rules that could not run.
- shared-path-resolution reports once per distinct value, not per point.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@arav-agarwal2
arav-agarwal2 merged commit 424ffe4 into main Oct 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant