Skip to content

Add bump-base-image skill and update golden value comparison#4733

Merged
balasaajay merged 2 commits into
NVIDIA:mainfrom
balasaajay:update-dependency-skill
May 12, 2026
Merged

Add bump-base-image skill and update golden value comparison#4733
balasaajay merged 2 commits into
NVIDIA:mainfrom
balasaajay:update-dependency-skill

Conversation

@balasaajay

Copy link
Copy Markdown
Contributor

What does this PR do ?

Add the bump-base-image skill capturing the end-to-end workflow for upgrading the NVIDIA PyTorch base container (both the GitHub docker/.ngc_version.dev pin and the GitLab .gitlab/stages/01.build.yml IMAGE_TYPE: dev rows), including the post-bump CI loop, golden-value refresh hand-off, and mr-broken triage path.

Simplify compare_golden_values_kl.py to a single per-(file, metric) signed average normalized relative difference (avg_rel_diff = mean((old − new) / old)), dropping KL / median / max-rel-diff, and update the update-golden-values skill (description, summary template, triage rules, gotchas) to match.

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact the @mcore-oncall.

Issue tracking

For PRs from open-source community contributors:

  • New features: a linked issue is required. Please open a feature request and reference it here before submitting the PR.
  • Small updates (bug fixes, minor improvements): a linked issue is recommended and will accelerate the PR review process.

Linked issue:

Contribution process

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!

All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.

Step 1: Mark PR as "Ready for Review"

  1. When your PR is ready, click Ready for Review.
  2. An oncall reviewer is auto-assigned and expert reviewers are notified based on your changes.
    • Some PRs may jump straight to step 2. This is determined by .github/CODEOWNERS.

⚠️ Only mark as ready once merge-conflicts are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

Step 2: Final Review

For PRs that change megatron/core, once all expert reviewers have approved, the Final Review label is applied automatically and final reviewers are assigned.

For PRs outside megatron/core, this step is skipped.

Step 3: Approved

Once all required reviewers have approved, the Approved label is applied automatically.

Merge

Any member of mcore-engineers will be able to merge your PR.

For MRs into `dev` branch The proposed review process for `dev` branch is under active discussion.

MRs are mergable after one approval by either eharper@nvidia.com or zijiey@nvidia.com.

balasaajay and others added 2 commits May 11, 2026 11:04
Document the end-to-end workflow for upgrading the NVIDIA PyTorch base
container (`nvcr.io/nvidia/pytorch:<YY.MM>-py3`) used by Megatron-LM CI,
distilled from PRs NVIDIA#4611 (the bump itself) and NVIDIA#4688 (the GitLab follow-up
that was needed because NVIDIA#4611 only touched the GitHub pin).

The skill encodes:

- The two pin sites that must move together: `docker/.ngc_version.dev`
  (GitHub CI, via `Dockerfile.ci.dev`) and the `IMAGE_TYPE: dev` rows of
  `.gitlab/stages/01.build.yml` (GitLab CI). A pre-merge `rg` snippet
  enforces the sync to prevent recurrence of the NVIDIA#4688 trap.
- The `Run functional tests` label, which routes the PR into SCOPE=L1 /
  N_REPEAT=5 / cadence-bypass in `.github/workflows/cicd-main.yml` so the
  first CI run already exercises the full functional matrix.
- The `copy-pr-bot` `/ok to test <sha>` flow for fork PRs: authorization
  is per-SHA, so every push needs a fresh comment.
- Hand-off to the `update-golden-values` skill for refreshing drifted
  goldens via `download_golden_values.py --only-failing`, with the KL
  summary becoming the PR description blurb (78 files in NVIDIA#4611).
- Triage for real regressions: file a tracking issue (e.g. NVIDIA#4654, NVIDIA#4657),
  flip the recipe's `scope:` from `[mr, mr-github]` to
  `[mr-broken, mr-github-broken]` with an inline comment, and let the
  bump merge on its own concern.

The workflow is resumable across CI rounds because state lives in the PR
itself, not the skill. A typical bump flows S1 (PR open, both pins
moved, label applied) -> CI -> S2 (classify failures) -> S3a (refresh
goldens) and/or S3b (flip scope to broken) -> CI -> ... -> S4 (Step 7
sync check) -> merge. Each invocation reads `git status`, the latest CI
run, and the recipe scopes to determine which state it's in; nothing is
carried in skill memory between sessions. Wall-clock "Day 1/2/3" is just
shorthand for "human pings the agent after each ~6h CI round".

The skill deliberately stays out of scope for: bumping LTS in the same
PR (separate cadence), hand-editing golden JSONs (use the dedicated
skill), and fixing real regressions inline with the bump (use
`mr-broken` + an issue instead).

Signed-off-by: Ajay Balasa <abalasa@nvidia.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Enhanced the `bump-base-image` skill documentation to clarify the workflow for updating the PyTorch base image, emphasizing the importance of synchronizing GitHub and GitLab CI pins.
- Updated the `update-golden-values` skill to reflect changes in the scoring method, now using average normalized relative differences instead of KL divergence for golden value comparisons.
- Modified the `compare_golden_values_kl.py` script to compute and report average relative differences, improving clarity and usability for users comparing golden values.

These changes aim to streamline the process of updating golden values and ensure accurate reporting of differences, enhancing the overall CI workflow.
@balasaajay balasaajay requested a review from ko3n1g May 11, 2026 18:23
@copy-pr-bot

copy-pr-bot Bot commented May 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@balasaajay

Copy link
Copy Markdown
Contributor Author

/ok to test 3110a17

@balasaajay

Copy link
Copy Markdown
Contributor Author

/ok to test 3110a17

@balasaajay balasaajay marked this pull request as ready for review May 11, 2026 22:39
@svcnvidia-nemo-ci svcnvidia-nemo-ci requested a review from a team May 11, 2026 22:39
@balasaajay balasaajay added this pull request to the merge queue May 11, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/25701539485

Merged via the queue into NVIDIA:main with commit f744215 May 12, 2026
71 of 72 checks passed
@balasaajay balasaajay deleted the update-dependency-skill branch May 12, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants