cli: full --help_all parity sweep with a permanent drift guard - #845
Merged
Conversation
Pure mechanical extraction — the parser-construction block moves verbatim into build_parser() so tests and tooling can walk the real argparse tree programmatically. No behavior change.
Parity suite that walks build_parser() and diffs every leaf command against
the hand-curated COMPLETE COMMAND REFERENCE, resolving the block notation
(= PARENT plus: inheritance, + GROUP includes). Per leaf it asserts:
1. every long option the parser accepts is documented in its block
2. every flag a block declares still exists on that parser leaf
3. documented --long/-short alias pairs match the parser's aliases
4. choice sets written as {a,b,c} in a block's own text match the parser
5. positionals (file|object selector, init/validate/history operands)
are documented
6. every parser action carries an argparse help string
7. the tree shows the real shape (training model rows, history show/rerun)
Currently 64 failures — the drift this sweep exists to close, e.g.:
- CORE_STD never gained --systemname, --quiet, --skip-ssh-check,
--skip-fs-separation-gate
- vectordb rework undocumented: --mode renamed --benchmark-mode, new
--vdb-engine/--vdb-index, distributed/sweep flag family missing
- checkpointing --model/-m flag never documented (tree presents a model
positional that does not exist)
- --accelerator-type short is -at, reference says -g
- --autoscaler-mode choices are {qos,capacity}, reference says
{qos,predictive}
- history subcommands are show/rerun, reference says list/replay
- --oversubscribe, --allow-run-as-root, --stream-log-level have no
argparse help string at all
… truth
Full sweep of every supported CLI argument against the hand-curated
COMPLETE COMMAND REFERENCE. The parity suite (previous commit) now passes
228/228. What changed:
Structure
- The tree no longer invents model positionals for checkpointing, vectordb,
and kvcache — checkpointing selects with --model/-m (required flag),
vectordb with --vdb-index, open/whatif kvcache with --model/-m
(default tiny-1b). Only training has a model positional. SYNOPSIS
rewritten per-benchmark to match.
- history subcommands corrected: show/rerun (were documented as
list/replay); blocks renamed HI_SHOW/HI_RERUN.
- validate and rules-coverage get proper VALIDATE / RULES_COVERAGE blocks.
Common groups
- CORE_STD gains --systemname/-sn, --quiet, --skip-ssh-check,
--skip-fs-separation-gate, plus the env-var fallbacks
(MLPERF_RESULTS_DIR / MLPERF_SYSTEMNAME).
- TIMESERIES scope corrected: vectordb and kvcache run commands have the
flags in ALL modes, not just open/whatif.
- OPEN_STD annotated as absent from kvcache datasize.
Per-block corrections (parser is the source of truth)
- --accelerator-type short flag is -at, not -g; required on configview too.
- training: --params/--param/-p exists in all modes (closed restricted
subset); --o-direct, --drop-caches-timeout-seconds documented;
stale --checkpoint-folder removed from TR_RUN (training no longer
takes it); --data-dir is argparse-optional (required at parse time
only for file-mode datagen/run).
- checkpointing: --model/-m and --num-processes/-np documented as
required everywhere incl. datasize/configview; --dlio-bin-path,
--exec-type, --checkpoint-subset, --o-direct on all modes.
- vectordb: rework documented — --mode renamed --benchmark-mode; new
--vdb-engine/--vdb-index; legacy --index-type kept (defaults to
--vdb-index); sweep-mode flags (--num-query-vectors, --search-limit,
--search-ef, --recall-k, --gt-collection, --vector-dim); storage
location flags (--storage-root/--storage-type); distributed-launch
family (--distributed, --hosts, --npernode, --mpi-impl,
--coordination, --rank-output-dir, --seed, --ready-timeout).
- kvcache: --num-users no longer exists in closed (run or datasize) and
is optional (default 100) in open run; --model/-m is an open/whatif
flag; new --config, --max-concurrent-allocs, --enable-latency-tracing;
--autoscaler-mode choices are {qos,capacity} not {qos,predictive};
closed-fixed values list corrected.
- validate: --mlperf-version default is v3.0, not v5.1.
- reportgen/lockfile: accept the full CORE_STD set (documented via
+ CORE_STD); lockfile requires a resolvable --results-dir.
argparse surface
- --oversubscribe, --allow-run-as-root, --stream-log-level finally get
help= strings (they rendered blank in every --help).
The examples predated the mode positional (closed/open/whatif), the training model positional, the file|object storage selector, and the --systemname requirement; history subcommands are show/rerun, and the closed accelerator/model sets changed. Also drop the nonexistent 'checkpointing validate' subcommand.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full sweep of every supported CLI argument against the
--help_allreference, plus a permanent parity test so the hand-curated page can no longer drift from the parser silently. Follow-up to #844, which fixed one instance of this drift (--checkpoint-subset); this closes the systemic gap.The sweep
A programmatic diff of the real argparse tree (all 48 leaf commands) against the reference found 64 classes of drift, including:
--systemname/-sn,--quiet,--skip-ssh-check,--skip-fs-separation-gate--model/-mflag (never documented anywhere in the reference), vectordb with--vdb-index, open/whatif kvcache with--model/-m--moderenamed to--benchmark-mode, new--vdb-engine/--vdb-index, the sweep-mode family (--recall-k,--gt-collection, …), storage-location flags, and the distributed-launch familyshow/rerun, documented aslist/replay--accelerator-type's short flag is-at, documented as-g--autoscaler-modechoices are{qos,capacity}, documented as{qos,predictive}--num-usersno longer exists in closed mode and is optional (default 100) in open run; the reference required it everywhere--checkpoint-folder; the reference required itvalidate --mlperf-versiondefault isv3.0, documented asv5.1--oversubscribe,--allow-run-as-root,--stream-log-levelhad no argparse help string at allThe permanent guard
tests/unit/test_help_all_parity.py(228 tests) walksbuild_parser()(extracted fromparse_arguments()as a pure refactor) and asserts, per leaf command:= PARENT plus:/+ GROUPnotation)--long/-shortalias pairs match the parser{a,b,c}match the parserfile|object,init/validate/historyoperands) are documentedCommit sequence is TDD RED→GREEN: the parity suite lands failing (64 failures documented in the commit message), then the reference rewrite turns it green.
Also updates the stale CLI examples in CLAUDE.md (pre-mode-positional shape).
Parser observations surfaced by the sweep (not changed here)
These are behaviors the sweep documented as-is; flagging in case any are unintended:
runand closed kvcacherunregister the TIMESERIES flags, contradictingadd_timeseries_arguments's docstring ("only called from open/whatif tier builders")--checkpoint-subsetis registered on checkpointingdatasizeandconfigview, not justrunlockfile generate/verifyrequire a resolvable--results-dir(post-parse gate), which is surprising for a pure utilityAll four CI suites pass locally:
tests(3392),mlpstorage_py/tests(941),kv_cache_benchmark/tests(238),vdb_benchmark/tests(228).