1st: feat(reportgen): populate v3.0 final results tables (all 8 tables)#826
Draft
FileSystemGuy wants to merge 14 commits into
Draft
1st: feat(reportgen): populate v3.0 final results tables (all 8 tables)#826FileSystemGuy wants to merge 14 commits into
FileSystemGuy wants to merge 14 commits into
Conversation
…F-a)
Assert reportgen emits the shared System-Under-Test block for each
workload row, sourced from system-description.yaml: sut_organization,
sut_name/sut_description as hyperlinks (HTML anchor in CSV, {text,href}
in JSON), sut_rus from total_rack_units, and blank manual-fill
placeholders. Guards the D-10 6-col prefix / D-12 trailing-issues
invariants. Fails until F-a is implemented.
reportgen now emits the v3.0 results-table SUT columns for every workload
row, sourced from the row's system-description.yaml:
- sut_organization (path), sut_rus (total_rack_units)
- sut_name / sut_description as Hyperlink cells -> the system-description
.yaml / .pdf (repo-root-relative href, Option A)
- sut_public_id / sut_type / sut_access_protocol / sut_availability /
sut_integrated_client_storage / sut_usable_capacity_tib as
present-but-blank placeholders (submitter fills manually)
New Hyperlink value type renders as an HTML anchor in results.csv (via
__str__) and a {text,href} object in results.json (via MLPSJsonEncoder;
write_json_file now passes cls=MLPSJsonEncoder). The sut_* group is
ordered right after the D-10 6-col prefix and before metric groups, so
the prefix stays first and 'issues' stays last.
Green: new test_reportgen_sut_block.py + all 4 CI suites pass.
Assert reportgen resolves each run's .mlps-code-image pointer to code-<hash8> and emits sut_code/sut_logs anchors -> closed/<org>/code-<hash8>/ (Logs a placeholder to the same dir). Missing pointer -> blank, no crash.
Adds sut_code / sut_logs to the SUT block. Each is resolved from the run leaf's .mlps-code-image pointer via the canonical code_image helpers (_read_pointer D-61, _pool_dir_name D-62) to a repo-root-relative anchor -> <category>/<orgname>/code-<hash8>/. Logs is a placeholder pointing at the same code-image dir for now (future change retargets it). Absent or malformed pointer -> blank cell, no crash. Green: 4 CI suites (2942/890/228/238) + test_reportgen_sut_block.py.
…lock (Slice-VDB-a)
vdb_summary module (build/write_vdb_summary): locate native query-phase
statistics.json (single-node root + MPI vectordb/<phase> subdirs), coerce
dict recall to scalar mean_recall, ignore the load phase. Module absent →
import error (RED).
VectorDBBenchmark.metadata['parameters'] must carry the reportgen VDB
identity keys (engine/index_type/num_vectors/dimension); combined_params
was never set so the block is {} → engine None (RED).
…Slice-VDB-a) Add mlpstorage_py/benchmarks/vdb_summary.py with pure build_vdb_summary / write_vdb_summary: locate the native query-phase statistics.json (single-node run root + MPI vectordb/<phase> subdirs), copy it whole, coerce dict recall to scalar mean_recall; the load phase is excluded. Directory-in, dict/path-out so reportgen can call it without constructing a VectorDBBenchmark. VectorDBBenchmark now sets self.combined_params (engine/index_type/ num_vectors/dimension) so base.get_metadata surfaces them in metadata['parameters'] — the VDB final-table identity columns reportgen reads. execute_run best-effort writes summary.json at the run root after a query run (skipped on --what-if; never fatal). GREEN for Slice-VDB-a.
reportgen _aggregate_vdb must gain: Read B/W (disk_io.total_bytes_read_per_sec ÷1024³ GiB/s), # Client Nodes (disk_io.host_count), Storage IOPs (present-but-blank, unmeasured), recall as percentage (×100, incl. dict→scalar coercion), in-memory legacy backfill from native statistics.json WITHOUT writing into the package, and identity columns (engine/index_type/num_vectors/ dimension) falling back to persisted metadata['args'] when parameters is empty (parameters win when present). BenchmarkRun.run_args must surface metadata['args'] (empty dict when absent). Existing TestVdbPassThrough recall assertions updated to the percentage form. 10 failing.
…ice-VDB-b) BenchmarkRunData gains run_args (persisted metadata['args'] snapshot), populated at all three extractor sites; BenchmarkRun.run_args exposes it. report_generator._aggregate_vdb now emits the v3.0 VDB final-table columns: - vdb_read_bw_gibps (disk_io.total_bytes_read_per_sec ÷ 1024**3, GiB/s) - vdb_num_client_nodes (disk_io.host_count) - vdb_storage_iops (present-but-blank; not measured) - vdb_recall as a percentage (×100), with dict→scalar mean_recall coercion on both the summary.json and recall_stats.json paths - vdb_num_vectors / vdb_dimension / vdb_engine / vdb_index_type via parameters-first, persisted-args-fallback (blank only when truly absent) New _load_vdb_summary backfills legacy packages IN MEMORY via vdb_summary.build_vdb_summary when summary.json is absent — never writing into the submission package. GREEN for Slice-VDB-b.
…VCache) The v3.0 kvcache final table carries a shared '# Client Nodes' column. Its source is the top-level host_count field kvcache's _write_run_summary already persists; reportgen now surfaces it as kvcache_num_client_nodes. The table's Read/Write B/W (GiB/s) and P95 Read Latency columns already flow verbatim via the existing per-option flattening: the per-option aggregated_read/write_bandwidth_gbps values are already GiB/s binary (kvcache cache.py divides bytes by 1024**3 despite the _gbps name), so no unit conversion is applied. host_count was the only missing field.
Read B/W (GiB/s), # Client Nodes, # Simulated Accelerators — sourced from summary.json top-level num_hosts/num_accelerators and DLIO's scalar train_io_mean_MB_per_second (MiB/s, per statscounter.py 'MiB/second' log).
Emits the v3.0 training final-table columns that the list-valued metric
aggregation did not carry, read straight from each run's summary.json
(the metadata-complete parse path drops them: system_info is never
reconstructed and scalar metrics are filtered out):
- train_num_client_nodes <- summary top-level num_hosts
- train_num_simulated_accelerators <- summary top-level num_accelerators
- train_read_bw_gibps <- mean of DLIO's per-run scalar
train_io_mean_MB_per_second across non-warmup runs, /1024
Read B/W unit: DLIO's train_io_mean_MB_per_second is MiB/s despite the
'MB' label (statscounter.py computes samples/s * record_size / 1024/1024
and logs it as 'MiB/second'), so GiB/s = value / 1024 (binary), matching
the vdb/kvcache slices. Blank-but-present when any run omits the scalar.
Updates the D-13 prefix invariant test: metric-mean columns keep
train_mean_of_; the new final-table columns keep only the train_ group
prefix (preserving column ordering) without mean_of_.
…pointing) Write/Read B/W (GiB/s, verbatim — already binary) + Write/Read Duration from DLIO scalars save_/load_checkpoint_io_mean_GB_per_second and _duration_mean_seconds; # Client Nodes from num_hosts; Checkpoint Mode from checkpoint.mode param; DP Instances = int(ClosedGPUs/GPUpDP).
…eckpointing)
Emits the v3.0 checkpointing final-table columns, read from each run's
summary.json (the metadata-complete parse path drops the DLIO scalar
metric keys via its list-only filter, and they are absent from the
fabricated fixtures):
- checkpoint_num_client_nodes <- summary top-level num_hosts
- checkpoint_mode <- checkpoint.mode param (subset->Subset,
default/absent->Full)
- checkpoint_dp_instances <- int(ClosedGPUs/GPUpDP) from
LLM_ALLOWED_VALUES[model]
- checkpoint_write_bw_gibps <- save_checkpoint_io_mean_GB_per_second
- checkpoint_write_duration_secs <- save_checkpoint_duration_mean_seconds
- checkpoint_read_bw_gibps <- load_checkpoint_io_mean_GB_per_second
- checkpoint_read_duration_secs <- load_checkpoint_duration_mean_seconds
B/W unit: DLIO's *_io_mean_GB_per_second is already GiB/s binary
(checkpoint_size in GiB / seconds, logged 'GiB/second'), so NO conversion
— unlike training's MiB-labelled figure. Blank-but-present when any run
omits a scalar. Existing checkpoint_mean_of_* columns (built on the
fabricated list fixture keys) are left untouched per the agreed scope.
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
FileSystemGuy
changed the title
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.
Summary
Makes
mlpstorage reports reportgenemit a data value for every column of the v3.0 user-visible final results tables (all 8 tables). This is the reportgen half of #823 — the tool computed most metrics already but never surfaced the columns the tables need.Built as one shared foundation + four per-workload metric slices, each landed RED→GREEN (TDD):
sut_*group: readssystem-description.yamlper row (solution.submission_name,total_rack_units), emits the 4 auto-filled SUT cells + 6 blank-but-present manual placeholders, and resolves theCode/Logshyperlinks from the run's code-image pointer. Hyperlinks render as<a href>in CSV and{text, href}in JSON.benchmarks/vdb_summary.py(purebuild_/write_vdb_summary); the VDB producer now writes a canonicalsummary.json, and reportgen backfills legacy packages in memory (never mutates a submission package). Emits Read B/W (GiB/s), # Client Nodes, Vector Count/Dimension, Index Type, Recall (%), Queries/s, Query Latency; Storage IOPs present-but-blank (not measured).# Client Nodesfromsummary['host_count']; the per-option B/W / P95-latency / throughput columns already flow verbatim.# Client Nodes,# Simulated Accelerators, andRead B/W (GiB/s).# Client Nodes, DP Instances, and Write/Read B/W (GiB/s) + Write/Read Duration (s).Units (all binary GiB/s, verified against DLIO / benchmark source)
kv_cache/cache.pycomputes(bytes / 1024³) / durationdespite the_gbpsfield name; no conversion.train_io_mean_MB_per_secondis really MiB/s (statscounter.pycomputessamples/s × record_size / 1024 / 1024and logs it "MiB/second"), so GiB/s = value ÷ 1024.*_io_mean_GB_per_secondis already GiB/s binary (checkpoint size in GiB ÷ seconds, logged "GiB/second"); no conversion.Notes for reviewers
results.{csv,json}.train_io_throughput_MB_per_secondas a list;checkpoint_read/write_throughput_GB_per_secondas lists with no durations). Real DLIO emits scalartrain_io_mean_MB_per_secondandsave/load_checkpoint_io_mean_GB_per_second+_duration_mean_seconds. This PR sources the new final-table columns from the real DLIO keys (read straight fromsummary.json, since the metadata-complete parse path filters scalars out); the pre-existingcheckpoint_mean_of_*columns built on the fabricated keys are left untouched. A separate cleanup could correct those fixtures and the existing training/checkpointing aggregation.Logshyperlink points at the code-image dir as a known placeholder pending a future logs-target change.Testing
All four CI suites pass locally (run separately per the conftest layout):
tests/mlpstorage_py/testsvdb_benchmark/testskv_cache_benchmark/testsEvery slice adds RED tests first, then the implementation. Draft while the reviewer weighs in on the fixture-cleanup follow-up and the
Logsplaceholder.