fix(validator): stop unenforced nccl-all-reduce-bw gates; wire eks+h200 by yuanchen8911 · Pull Request #1338 · NVIDIA/aicr

yuanchen8911 · 2026-06-12T19:01:58Z

Summary

Stops three nccl-all-reduce-bw performance gates that were declared in recipes but silently skipped by the validator (false "passed" without measuring). Removes the two that can't be enforced without a new testbed-validated template, and wires the one (eks+h200) that reuses the existing H100/EFA template.

Motivation / Context

The performance validator resolves NCCL support via an exact (variant, service) → []accelerator table with no Any fallback, so any declared tuple absent from the table returns skipped. A skipped check does not fail, so the performance phase reports passed without ever measuring or enforcing the threshold. An audit of all recipes declaring nccl-all-reduce-bw* found three affected tuples.

Fixes: #1337
Related: N/A

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Refactoring (no functional changes)
Build/CI/tooling

Component(s) Affected

Recipe engine / data (pkg/recipe)
Validator (pkg/validator)
Docs/examples (docs/, examples/)

Implementation Notes

Audit result (every recipe with an active NCCL check):

Recipe	Tuple	Validator	Resolution
`b200-gke-cos-training` (+ Kubeflow child)	`gke+b200`	skipped	remove — A4 uses GPUDirect RDMA over RoCE/CX-7; the only GKE template is TCPXO/FastRak-specific to a3-megagpu H100, and the GKE Go path is hardwired to TCPXO 8-NIC discovery. Needs a new testbed-validated template.
`h100-aks-training` (+ ubuntu child)	`aks+h100`	skipped	remove — no AKS template or scheduling path exists at all.
`h200-eks-training`	`eks+h200`	skipped	wire — H200 is Hopper on EFA, electrically identical to H100 for NCCL.
`gb200-eks-training`, `gb200-oke-training`, `h100-eks-training`, `h100-gke-cos-training`	—	enforced	no change

Wiring for eks+h200: added H200 to variantDefault.EKS, added a NVIDIA-H200- prefix product matcher, and added testdata/h200/eks/runtime.yaml (mirrors the H100 EKS template). Recipe keeps its conservative >= 300 GB/s floor (loose for H200's HBM3e envelope; wants one EFA-testbed confirmation to tighten). Removed tuples are replaced with comments documenting what wiring each would require.

Testing

go test -race ./validators/performance/   # ok
go test ./pkg/recipe/...                   # ok
golangci-lint run -c .golangci.yaml ./validators/performance/...  # 0 issues

TestSupportedNCCLCombinationsHaveRuntimeTemplates (wiring guard) now covers h200/eks; added TestTemplatePath and TestAcceleratorProductMatchers cases for H200 and updated the variantDefault.EKS assertion. Full make qualify still pending — PR is WIP/draft.

Risk Assessment

Low — Isolated change, well-tested, easy to revert

Rollout notes: The two removed checks were no-ops (already reporting passed without measuring), so removal changes no observed behavior. The H200 wiring activates a check that previously skipped; the floor is conservative.

Checklist

Tests pass locally (make test with -race)
Linter passes (make lint) — scoped to changed package
I did not skip/disable tests to make CI green
I added/updated tests for new functionality
I updated docs if user-facing behavior changed
Changes follow existing patterns in the codebase
Commits are cryptographically signed (git commit -S)

github-actions · 2026-06-12T19:04:15Z

Recipe evidence check

Affected leaf overlays: 5

Recipe	Pointer	Verify	Digest match
`b200-gke-cos-training-kubeflow`	⚠️ missing	—	—
`b200-gke-cos-training`	⚠️ missing	—	—
`h100-aks-training`	⚠️ missing	—	—
`h100-aks-ubuntu-training-kubeflow`	⚠️ missing	—	—
`h100-aks-ubuntu-training`	⚠️ missing	—	—

How to refresh evidence

Run on a cluster matching the recipe's criteria:

aicr snapshot -o snapshot.yaml
aicr validate \
  -r recipes/overlays/<slug>.yaml \
  -s snapshot.yaml \
  --emit-attestation ./out \
  --push ghcr.io/<your-fork>/aicr-evidence
cp ./out/pointer.yaml recipes/evidence/<slug>.yaml

This gate is warning-only and never blocks merge. See ADR-007 for the trust model.

coderabbitai · 2026-06-12T19:06:28Z

📝 Walkthrough

Walkthrough

This PR adds EKS/H200 support to the NCCL all-reduce bandwidth validator (product matcher and supported-combination entry), adds an EKS H200 TrainingRuntime test manifest, updates related unit tests and template mapping, removes performance validation phases from AKS H100 and GKE B200 recipe overlays (replacing them with explanatory comments), and updates recipe-health and validation docs to reflect the changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

nccl-all-reduce-bw declared but skipped for gke+b200, aks+h100, eks+h200 (gates never enforced) #1337: Implements the same changes tracked in issue 1337—add H200 EKS support, add H200 testdata/runtime, and remove unsupported NCCL checks from B200/GKE and H100/AKS recipes.

Possibly related PRs

NVIDIA/aicr#1319: Modifies NCCL validator template/path handling for specific EKS accelerators (related area).
NVIDIA/aicr#1328: Overlaps validator/test changes for NCCL variant/default handling and template mapping.
NVIDIA/aicr#1233: Touches supported-combinations/template-selection logic in the NCCL validator (different target accelerators).

Suggested labels

theme/validation, size/M

Suggested reviewers

xdu31
mchmarny
ayuskauskas

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description thoroughly documents the motivation, implementation, testing, and risk assessment, directly relating to the changeset that removes two unenforceable NCCL checks and wires EKS+H200 support.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the main changes: stopping unenforced NCCL all-reduce-bw gates and wiring EKS+H200 support, which are the core objectives throughout the changeset.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

The performance validator resolves nccl-all-reduce-bw support via an exact (variant, service) -> []accelerator table with no Any fallback, so a recipe declaring the check for a tuple absent from the table silently skips. A skipped check does not fail, so the performance phase reports passed without measuring or enforcing the threshold (false assurance). Three tuples were affected: - gke+b200 (b200-gke-cos-training, Kubeflow child inherits) - aks+h100 (h100-aks-training, ubuntu child inherits) - eks+h200 (h200-eks-training) gke+b200 and aks+h100 cannot be enforced without a new, testbed-validated transport-specific runtime template (A4 RoCE/RDMA; AKS has no template or scheduling path at all), so their unenforced checks are removed and replaced with a comment documenting what wiring would require. eks+h200 is Hopper on EFA, electrically identical to h100 for NCCL, so it is wired: H200 added to variantDefault.EKS, a NVIDIA-H200- product matcher added, and testdata/h200/eks/runtime.yaml added (mirrors the H100 EKS template). The recipe keeps its conservative >= 300 GB/s floor. Refs NVIDIA#1337

mchmarny

Clean, well-reasoned fix. Removing the two silently-skipped gates is the fail-closed-correct move — they were reporting passed without ever measuring, so this removes false assurance rather than masking it. The eks+h200 wiring is sound: H200 reuses the H100/EFA template, the product matcher and template path are unit-tested, and h200-eks-training already declares the >= 300 floor (with an honest "loose floor" comment), so the validator change activates a real constraint.

A couple of non-blocking notes:

The PR body says "Full make qualify still pending — PR is WIP/draft," but it's marked ready with reviewers requested. CI is green (covers most of qualify) — just confirm make qualify ran clean before merge.
recipe-health.md going 32→37 also absorbs five pre-existing a100-* recipes whose health doc was stale on main — that's a legitimate drift-fix from regeneration, not new data, but worth a one-line note in the PR description so reviewers don't read it as scope creep.

Nothing blocks merge.

mchmarny · 2026-06-12T19:50:42Z

+// EFA — electrically identical to H100 for NCCL — so the two EKS runtime
+// templates must stay byte-identical apart from their leading comment/license
+// headers. This converts the comment-only guarantee into an enforced one: an
+// EFA-related edit to one template that is not mirrored in the other fails here


Nice — this converts the "keep in sync with H100" header comment into an enforced guard, so an unmirrored EFA edit fails here instead of silently diverging. Good way to make a 209-line duplicate safe.

github-actions Bot added area/recipes area/docs size/L labels Jun 12, 2026

yuanchen8911 added the area/validator label Jun 12, 2026

yuanchen8911 mentioned this pull request Jun 12, 2026

nccl-all-reduce-bw declared but skipped for gke+b200, aks+h100, eks+h200 (gates never enforced) #1337

Closed

yuanchen8911 force-pushed the fix/nccl-gke-b200-aks-h100-1337 branch from e24939a to b6725d5 Compare June 12, 2026 19:10

github-actions Bot removed the area/validator label Jun 12, 2026

yuanchen8911 force-pushed the fix/nccl-gke-b200-aks-h100-1337 branch 2 times, most recently from 5ce02c0 to 4cd27e0 Compare June 12, 2026 19:16

yuanchen8911 force-pushed the fix/nccl-gke-b200-aks-h100-1337 branch from 4cd27e0 to 27e1736 Compare June 12, 2026 19:25

yuanchen8911 changed the title ~~WIP: fix(validator): stop unenforced nccl-all-reduce-bw gates; wire eks+h200~~ fix(validator): stop unenforced nccl-all-reduce-bw gates; wire eks+h200 Jun 12, 2026

yuanchen8911 marked this pull request as ready for review June 12, 2026 19:30

yuanchen8911 requested review from a team as code owners June 12, 2026 19:30

yuanchen8911 requested review from mchmarny and xdu31 June 12, 2026 19:30

mchmarny approved these changes Jun 12, 2026

View reviewed changes

mchmarny merged commit d68eada into NVIDIA:main Jun 12, 2026
122 checks passed

coderabbitai Bot mentioned this pull request Jun 22, 2026

WIP: feat(recipes): GB300 EKS service-bound overlays (re-file of #1319) #1336

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(validator): stop unenforced nccl-all-reduce-bw gates; wire eks+h200#1338

fix(validator): stop unenforced nccl-all-reduce-bw gates; wire eks+h200#1338
mchmarny merged 1 commit into
NVIDIA:mainfrom
yuanchen8911:fix/nccl-gke-b200-aks-h100-1337

yuanchen8911 commented Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

mchmarny left a comment

Uh oh!

mchmarny Jun 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

yuanchen8911 commented Jun 12, 2026

Summary

Motivation / Context

Type of Change

Component(s) Affected

Implementation Notes

Testing

Risk Assessment

Checklist

Uh oh!

github-actions Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Recipe evidence check

How to refresh evidence

Uh oh!

coderabbitai Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

mchmarny left a comment

Choose a reason for hiding this comment

Uh oh!

mchmarny Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 12, 2026 •

edited

Loading

coderabbitai Bot commented Jun 12, 2026 •

edited

Loading