refactor(nvml-mock): consolidate NVLink bandwidth to bandwidth_per_link_mbps#405
refactor(nvml-mock): consolidate NVLink bandwidth to bandwidth_per_link_mbps#405giuliocalzo wants to merge 2 commits into
Conversation
giuliocalzo
force-pushed
the
nvml-mock/consolidate-nvlink-bandwidth-mbps
branch
from
7080c57 to
b164364
Compare
ArangoGutierrez
left a comment
ArangoGutierrez
left a comment
There was a problem hiding this comment.
The NVLink consolidation itself looks clean — bandwidth math checks out (per-link x2 x links reproduces the bidir aggregates) and the switch-to-if in topology.go is correct now that the fallback is gone. One thing to sort before merge: commit 09caf03 (the SIGPIPE fix) is identical to all of #409 — same edits to the same three validate-*.sh files. Whichever lands second will conflict or no-op, and it's an unrelated e2e fix in an NVLink refactor. Could you drop 09caf03 here and let #409 carry it? Happy to approve once that's split out.
|
409 was merged now this one needs rebase before merge |
giuliocalzo
force-pushed
the
nvml-mock/consolidate-nvlink-bandwidth-mbps
branch
from
b74c0a7 to
8670f2b
Compare
…nk_mbps NVLinkConfig exposed two overlapping bandwidth fields (bandwidth_per_link_gbps and bandwidth_per_link_mbps), where the engine preferred _mbps and only fell back to _gbps. The whole-GB/s field could not express NVLink5's 53.125 GB/s and was dead weight on gb200/gb300. Collapse to a single canonical bandwidth_per_link_mbps: - Remove BandwidthPerLinkGBPS from NVLinkConfig and simplify the engine bandwidth selection in topology.go. - Convert the _gbps-only profiles to _mbps (a100/h100 50->50000, b200 100->100000) and drop the redundant _gbps fallback on gb200/gb300. - Update docs/configuration.md and the configs/ mirror. - Update unit tests and regenerate Helm snapshots. Closes NVIDIA#404 Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The a100/h100/b200 profiles previously used the bidirectional-per-link marketing figure (50/50/100 GB/s) rather than the per-link unidirectional line rate that `nvidia-smi nvlink -s` actually reports. Align them with the gb200/gb300 convention: - a100 (NVLink3): 25781 Mbps (25.781 GB/s/link) - h100 (NVLink4): 26562 Mbps (26.562 GB/s/link) - b200 (NVLink5): 53125 Mbps (53.125 GB/s/link, same silicon as gb200) Per-link x 2 x links still reproduces the marketed bidirectional aggregates (~600 GB/s, ~900 GB/s, ~1.8 TB/s). Update the docs example to the NVLink4 rate and regenerate Helm snapshots. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
giuliocalzo
force-pushed
the
nvml-mock/consolidate-nvlink-bandwidth-mbps
branch
from
8670f2b to
6d73669
Compare
|
@ArangoGutierrez rebased onto the latest
CI is fully green ✅ (all 30 checks pass, including the complete |
2 participants
Summary
NVLinkConfigexposed two overlapping bandwidth fields —bandwidth_per_link_gbps(whole GB/s) andbandwidth_per_link_mbps(Mbps). The engine preferred_mbpsand only fell back to_gbps, so the whole-GB/s field was dead weight (it can't express NVLink5's 53.125 GB/s) and just added noise ongb200/gb300.This consolidates to a single canonical
bandwidth_per_link_mbps:BandwidthPerLinkGBPSfromNVLinkConfigand simplify the bandwidth selection intopology.go._gbps-only profiles to_mbpsand drop the redundant_gbps: 53fallback line fromgb200/gb300(they already set_mbps: 53125).profiles/and thepkg/gpu/mocknvml/configs/mirror, plusdocs/configuration.md.Realistic per-link speeds
While consolidating, the
a100/h100/b200profiles were found to use the bidirectional-per-link marketing figure (50/50/100 GB/s) instead of the per-link unidirectional line rate thatnvidia-smi nvlink -sactually reports (the semantics the field documents and thatgb200/gb300already follow). Corrected to:bandwidth_per_link_mbps25781(25.781 GB/s)26562(26.562 GB/s)53125(53.125 GB/s)53125(unchanged)53125(unchanged)These match the documented
nvidia-smi nvlink -svalues (e.g. GV100/A10025.781 GB/s, H10026.562 GB/s). The docs example was bumped to the NVLink4 rate accordingly.Closes #404. Deferred from #387 to keep that PR focused.
Test plan
go build ./...go test ./pkg/gpu/mocknvml/...golangci-lint run(0 issues)helm unittest deployments/nvml-mock/helm/nvml-mock