Repository navigation
fix: pin DSV4.1 FPM data to schema-valid HF revision - #345
Conversation
HF dataset PR #13 review found H200/B200 configuration manifests violating configuration-manifest-v3. Commit 38f41571 fixes the manifests; pinned tables, metadata and systems YAML are byte-identical.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: ai-dynamo/aisimulate/.coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)Preserve the Rust single oracle: Python may describe operations, load raw data, orchestrate, and present results, but must not compute per-op performance values.⚙️ CodeRabbit configuration file Files:
Read REVIEW.md before commenting.⚙️ CodeRabbit configuration file Files:
Source excerpt: Do NOT add Python-side interpolation, roofline/SOL formulas, empirical-utilization estimates, or per-call table lookups anywhere under `python/aisimulate/src/aisimulate_core/sdk/` (banned def shapes: the `_query_*` and `_loo...📄 CodeRabbit inference engine (python/aisimulate/.claude/rules/rust-core/parity.md) Files:
Before making any change under: `python/aisimulate/src/aisimulate/generator/**` MUST read: `python/aisimulate/.claude/rules/generator-development.md` Before making any change under `python/aisimulate/collector/**` MUST read: `python/aisimul...📄 CodeRabbit inference engine (AGENTS.md) Files:
Source excerpt: Only workflows under the repository-root `.github/workflows/` run for this repository.📄 CodeRabbit inference engine (REVIEW.md) Files:
📝 SummaryRisk: Low. The three areas for human attention are: whether the new HF revision contains the intended schema fixes; whether all pinned dataset files remain byte-identical; and whether downstream consumers can use the new revision. Change and contract: Evidence supplied: The PR description reports that the new revision is the squash merge of HF dataset PR Evidence missing and merge readiness: The changed manifest supports the pin update, but this evidence does not independently establish the remote dataset contents, schema fixes, file hashes, or test results. No current review findings or severity counts are supplied. Technical scope is limited to one revision value; merge readiness still depends on verifying the remote revision and the reported validation evidence. WalkthroughThe dataset manifest’s top-level revision value changed. Profile entries and file references remain unchanged. ChangesDataset manifest update
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The dataset snapshot pin is updated without changing the profiles or artifact references. The reported fresh-cache downloads and checksum checks passed, and no actionable merge-blocking risk is evident. 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
Full details: Modeling And Data EvidenceExplanation The PR changes a revision for FPM performance tables, but it supplies only provenance and binary validation. The reviewed diff changes one HF commit; all six profile entries and all 18 file SHA256 values remain unchanged. The reported validation downloads files and checks hashes. The loader and tests also verify paths, metadata presence, and cache integrity, not performance values. The PR supplies no reproducible old-versus-new data comparison, explained numerical golden diff, independent oracle, or machine-readable anomaly summary. The existing README held-out MAPE table is unchanged repository context, not evidence for this revision change. Resolution Add a reproducible before/after check for the old and new HF revisions. Download every pinned artifact from both commits, compare file contents and parsed performance tables, and publish machine-readable results with row counts, schema checks, finite-value/range checks, and key summary statistics. If the tables are identical, record that result explicitly and separately report the schema/provenance changes that motivated the new pin.
Comment |
HF dataset PR #13 merged as 78d29cfb, whose tree is identical to 38f41571.
88c9989 to
42b07cb
Compare
|
/ok to test 2b07cb |
|
/ok to test 42b07cb |
2 participants
What changes
Moves the DSV4.1 FPM data pin in
four_gpu_hf_dataset.jsonfrom73fdad29…to HFmaincommit78d29cfb5ef6865c99ccbd99870231659aac7ed8. That is the squash merge of HF dataset PR #13, and its tree is identical to the fixed PR head38f41571.Review of HF PR #13 found that the H200/B200 TP4 configuration and measurement manifests violate the published v3/v4 schemas. The same was true of the earlier PR158/PR232 DSV4.1 manifests. The HF fix:
aisim-commit-unknown-<id>-prNNNand moves the PR158 history leaves to match,producer_revisions,collected_byandcollector_attribution,manage_dataset.py validate,Every file this repository pins (tables, metadata sidecars, systems YAML) is byte-identical, so only the revision changes. Current
mainkeeps working at the old pin.Validation
{gb200,h200,b200}-tp4-{full,decoder_bounded}) downloaded from78d29cfbinto a fresh cache viapython -m aisimulate_core.sdk.fpm_dataset, with every SHA256 matching.tests/unit/sdk/test_fpm_dataset.pyandtests/cross_package/test_import_contract.py: 118 passed.