Skip to content

fix(storage#795): align datasize output and fail-fast on flat storage_options typos#797

Merged
FileSystemGuy merged 2 commits into
mainfrom
fix/issue-795-datasize-discrepancy
Jul 15, 2026
Merged

fix(storage#795): align datasize output and fail-fast on flat storage_options typos#797
FileSystemGuy merged 2 commits into
mainfrom
fix/issue-795-datasize-discrepancy

Conversation

@FileSystemGuy

Copy link
Copy Markdown
Contributor

Closes #795.

Summary

Reporter ran mlpstorage open training retinanet datasize for a b200/15GiB host and saw the recommendation "Number of training files: 257,173" — but the log line one second earlier said "skip_listing enabled: 1,170,301 train files", and the emitted datagen-command hint bundled num_files_train=257173 with listing_validation_interval=1000 (right bucket for 1.17M, wrong for 257K). Their later run also failed with Disallowed parameter override: storage.storage_library = s3dlio when they tried to pass the storage backend explicitly.

Three related bugs, one PR:

  1. Datasize log/hint discrepancy: _apply_skip_listing_params ran in __init__ before datasize() computed its recommendation, so it read num_files_train from the workload YAML default (retinanet_b200.yaml: 1170301). Moved the call to fire in datasize() after the DRAM-derived recommendation lands in params_dict, threaded through as num_files_override. Now the INFO log, the Number of training files: result line, and the emitted datagen-hint's interval all agree.

  2. Emitted datagen-hint inconsistency: same root cause — the hint's listing_validation_interval came from the pre-computed injection. Fixed by (1).

  3. Flat storage.storage_library typo UX: the flat form isn't a real DLIO config path; the tool auto-injects storage.storage_options.storage_library from STORAGE_LIBRARY env in --object mode, so the user's --params had zero effect on the run. But the CLOSED disallowed-override check rejected it with a message that didn't hint at the correct spelling. Added mlpstorage_py/rules/param_hints.py with a KNOWN_PARAM_TYPOS map (three known-typo entries: storage.storage_library, storage.uri_scheme, storage.prefetch_window), a fail-fast gate in cli_parser.update_args so the user doesn't wait through MPI cluster collection just to be told the param was wrong, and a "Did you mean X?" hint appended to the runtime and submission-checker (3.6.2 CLOSED, 3.6.3 OPEN) disallowed-override messages so mlpstorage validate on stored submissions gives the same suggestion.

Backwards compatibility

  • No new CLI flag. num_files_override is an internal Python parameter used only by datasize().
  • Existing runs are unaffected. The datasize change is display-side only; the runtime and submission-checker file-count rules re-derive their thresholds from run metadata, not from anything the log or hint said.
  • Comparability across the fix boundary is unchanged. listing_validation_interval controls startup HEAD-check density, not measured throughput; both values pre/post fix are TOOL_INJECTED_PARAMS and don't gate any rule.

Test plan

  • uv run pytest tests — 2925 passed, 1 skipped, 13 deselected (+9 new)
  • uv run pytest mlpstorage_py/tests — 866 passed
  • uv run pytest vdb_benchmark/tests — 174 passed
  • uv run pytest kv_cache_benchmark/tests — 238 passed

New tests:

  • tests/unit/test_skip_listing_datasize_interval.py (3 tests) — override wins over combined_params for the 257,173 recommendation, no-override still falls back to combined_params for non-datasize commands, user-supplied --params dataset.listing_validation_interval=N still short-circuits.
  • tests/unit/test_param_typo_hints.py (9 tests) — typo-map integrity, CLI-parse fail-fast for the reporter's exact command shape, multiple typos surfaced in one error, canonical nested form still accepted, runtime check appends hint, unknown-typo message unchanged.

…_options typos

Fixes three bugs surfaced by the storage#795 reporter:

1. Datasize INFO log used the workload YAML default (retinanet_b200:
   1,170,301) for the "skip_listing enabled: N train files" message,
   contradicting the "Number of training files: 257,173" result line
   printed a moment later. Move the _apply_skip_listing_params call
   from __init__ to datasize() where the DRAM-derived recommendation is
   authoritative.

2. Emitted datagen-command hint bundled dataset.num_files_train=257173
   with dataset.listing_validation_interval=1000 — the interval bucket
   is right for the YAML default, wrong for the recommendation. Both
   values now agree because the same deferred call sets the interval.

3. Flat --params storage.storage_library=s3dlio (a typo of
   storage.storage_options.storage_library) had no effect on the run
   (the tool auto-injects the nested form) but tripped the CLOSED
   disallowed-override check with a message that didn't point at the
   correct spelling. Add a KNOWN_PARAM_TYPOS map and fail fast at CLI
   parse time; append the "Did you mean X?" hint to the runtime and
   submission-checker (3.6.2, 3.6.3) disallowed-override messages so
   old submissions get the suggestion at validate time too.

Behavior for non-datasize commands is unchanged: _apply_skip_listing_params
still reads combined_params on the run/datagen/configview/reportgen path.
No new CLI flag — num_files_override is an internal parameter used only by
datasize().
@FileSystemGuy
FileSystemGuy requested a review from a team July 15, 2026 19:27
@github-actions

Copy link
Copy Markdown

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

@FileSystemGuy
FileSystemGuy merged commit 79d926c into main Jul 15, 2026
4 checks passed
@FileSystemGuy
FileSystemGuy deleted the fix/issue-795-datasize-discrepancy branch July 15, 2026 19:30
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.

Data size requirement discrepancy between the datasize and training command

1 participant