Skip to content

Add ModelOpt QAD skill for Slurm workflows - #2010

Open
mxinO wants to merge 9 commits into
mainfrom
mxin/qad-skill
Open

Add ModelOpt QAD skill for Slurm workflows#2010
mxinO wants to merge 9 commits into
mainfrom
mxin/qad-skill

Conversation

@mxinO

@mxinO mxinO commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: new feature

Adds a general Slurm-only QAD skill based on the supported Megatron Bridge
workflow. The skill:

  • starts from a measured BF16-to-PTQ benchmark gap and preserves the preceding
    PTQ configuration or recipe;
  • gates QAD on exact Megatron Bridge model support and successful Megatron PTQ,
    using its master-rank quantizer summary as a scoped amax sanity check;
  • requires model- and hardware-derived TP/PP/CP/EP/ETP topology selection;
  • streams and randomly samples only the required
    nvidia/Nemotron-Cascade-2-SFT-Data token budget and uses Megatron sequence
    packing;
  • defaults to 32K sequences, LR 1e-5 with cosine decay, a 1000-step cap, and
    GBS 512;
  • requires explicit user authorization because QAD is costly, validates two
    batches every 25 steps, saves every 50 steps, and monitors a decreasing
    smoothed loss trend;
  • evaluates an early checkpoint around step 150 and continues only when
    benchmark recovery and the loss trend justify more training;
  • follows the established common Slurm and remote-execution guidance instead of
    duplicating mutable commands from the Megatron Bridge README.

Also exposes Megatron Bridge save_interval, exit_interval, and
exit_duration_in_mins through examples/megatron_bridge/distill.py, with example-test coverage for checkpoint
and ModelOpt-state preservation at an early exit.

Usage

Use the QAD skill to recover the measured BF16-to-PTQ benchmark gap for
<model> on <Slurm cluster>, preserving the validated PTQ recipe.

Testing

  • PYTHONPATH=$PWD pre-commit run --all-files
    • Passed every hook on the rebased branch, including Ruff, Ruff format, mypy,
      YAML/recipe validation, launcher reference validation, Bandit, generated
      arguments, symlink synchronization, and Markdown lint.
  • python ~/.codex/skills/.system/skill-creator/scripts/quick_validate.py .agents/skills/qad
    • Skill is valid!

Qwen3-0.6B result-bearing validation:

  • Resources: one exclusive node, 8 H100 GPUs

  • Container: nvcr.io/nvidia/nemo:26.06

  • Quantization: NVFP4, group size 16, embedding excluded

  • QAD topology: TP=1, PP=1, CP=4, EP=1, DP=2

  • Training validation configuration: sequence length 32768, MBS=1, GBS=8,
    train_iters=1000, LR 1e-5 / minimum LR 1e-6, 50 warmup iterations,
    cosine decay, eval_interval=150, exit_interval=150,
    exit_duration_in_mins=220

  • This result-bearing run used the then-current coupled eval/save cadence. The
    final skill now validates two batches every 25 steps and saves every 50; the
    example test covers the independent checkpoint cadence.

  • The reduced GBS 8 is intentionally validation-only; the skill retains GBS 512
    as the production default.

  • Data: exactly 10,000,000 sampled tokens from four
    nvidia/Nemotron-Cascade-2-SFT-Data configs:

    • math: 2,306,011 tokens / 364 documents
    • science: 1,191,257 tokens / 285 documents
    • chat: 6,142,077 tokens / 1,800 documents
    • instruction following: 360,655 tokens / 411 documents
  • Megatron built packed 32K GPT samples from the materialized prefixes; the full
    dataset was not downloaded.

  • QAD loss was finite and decreased from 0.2640341 at iteration 10 to
    0.1060580 at iteration 150. Final gradient norm was 0.747, with zero
    skipped and zero NaN iterations. Validation distillation loss was
    0.09715855.

  • The iteration-150 checkpoint saved successfully with modelopt_state, and
    both PTQ and QAD-150 exported to unified Hugging Face format.

  • Identical full MMLU 0-shot comparison through the Megatron evaluator:

    Model Accuracy
    BF16 0.39517164
    PTQ 0.32851446
    QAD-150 0.38740921

    QAD-150 recovered 0.05889475 / 0.06665718 = 88.35% of the measured PTQ gap,
    so validation stopped at the early evidence gate rather than continuing
    blindly toward 1000 iterations.

Before your PR is "Ready for review"

  • Is this change backward compatible?: ✅
  • If you copied code from any other sources or added a new PIP dependency, did
    you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: ✅
  • Did you update Changelog?: N/A — this adds an agent skill and example-only
    lifecycle flags.
  • Did you get Claude approval on this PR?: N/A

Additional Information

All seven branch commits are cryptographically signed and include a
Signed-off-by trailer.

Summary by CodeRabbit

  • New Features

    • Added comprehensive guidance for running quantization-aware distillation workflows, including staged execution, data preparation, evaluation, recovery, and reporting.
    • Added a Nemotron-Cascade-2 dataset blending configuration with weighted sources.
  • Enhancements

    • Distillation runs can now save checkpoints and exit based on iteration counts or elapsed time.
    • Checkpoint saving defaults now align with evaluation intervals when no custom interval is provided.
  • Tests

    • Improved validation of early-exit behavior and checkpoint creation.

mxinO added 5 commits July 23, 2026 13:03
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a QAD skill specification, a Nemotron-Cascade-2 blend configuration, a shared skill symlink, and configurable distillation checkpoint and early-exit controls with strengthened QAD test assertions.

Changes

QAD workflow support

Layer / File(s) Summary
QAD workflow definition
.agents/skills/qad/SKILL.md
Defines PTQ-gap validation, Slurm execution, topology selection, token-budgeted data preparation, staged QAD runs, monitoring, recovery, and reporting requirements.
Token-budgeted data preparation
.agents/skills/qad/assets/nemotron-cascade-2-blend.yaml, .claude/skills/qad
Adds the weighted Nemotron-Cascade-2 dataset blend configuration and links the Claude skill path to the shared QAD skill.
Distillation checkpoint and exit controls
examples/megatron_bridge/distill.py, tests/examples/megatron_bridge/test_qad.py
Adds configurable save and exit intervals, wires them into training, and verifies the resulting checkpoint iteration and directory.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: aanoosheh, benchislett, fridah-nv

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a ModelOpt QAD skill for Slurm-based workflows.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Security Anti-Patterns ✅ Passed Changed Python files avoid the banned patterns: no hardcoded trust_remote_code=True, no unsafe torch/numpy loads, no eval/exec, and no #nosec or new deps.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mxin/qad-skill

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.51%. Comparing base (33d05b0) to head (95e29fa).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2010      +/-   ##
==========================================
+ Coverage   66.83%   75.51%   +8.67%     
==========================================
  Files         519      519              
  Lines       58916    58916              
==========================================
+ Hits        39376    44488    +5112     
+ Misses      19540    14428    -5112     
Flag Coverage Δ
examples 43.18% <ø> (-0.20%) ⬇️
unit 54.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mxinO added 4 commits July 23, 2026 13:37
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Meng Xin <mxin@nvidia.com>

# Conflicts:
#	examples/megatron_bridge/distill.py
Signed-off-by: Meng Xin <mxin@nvidia.com>
@mxinO
mxinO marked this pull request as ready for review July 28, 2026 01:10
@mxinO
mxinO requested review from a team as code owners July 28, 2026 01:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.

Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.

👉 Steps to fix this

Actionable comments posted: 2

🧹 Nitpick comments (1)
.agents/skills/qad/assets/nemotron-cascade-2-blend.yaml (1)

5-43: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Pin the dataset revision for reproducible data.

All entries use the floating nvidia/Nemotron-Cascade-2-SFT-Data identifier without a revision, while .agents/skills/qad/SKILL.md Lines 206-209 require reporting an exact source revision. Pin the revision in the blend schema if supported, or capture and validate the resolved revision in the generated artifact.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/qad/assets/nemotron-cascade-2-blend.yaml around lines 5 - 43,
Pin every nvidia/Nemotron-Cascade-2-SFT-Data entry in the blend configuration to
an immutable dataset revision using the schema’s supported revision field. If
the blend schema cannot express revisions, update the generation flow to record
and validate the resolved revision in the output artifact, preserving the
requirement from SKILL.md for an exact source revision.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/qad/SKILL.md:
- Around line 160-164: Update the initial-stage instructions around the training
configuration and the QAD-150 export/evaluation steps to record the actual exit
iteration, distinguish duration-triggered exits before iteration 150, and run
the 150-step gate only when checkpoint 150 exists; otherwise mark the stage
incomplete and direct the operator to resume or relaunch instead of labeling an
earlier checkpoint as QAD-150.

In `@examples/megatron_bridge/distill.py`:
- Around line 164-184: Validate eval_interval, eval_iters, save_interval,
exit_interval, and exit_duration_in_mins immediately after CLI parsing,
rejecting zero or negative values before they reach scheduling or validation
logic; if any zero value is intentionally supported as a disable sentinel,
handle it explicitly and consistently instead.

---

Nitpick comments:
In @.agents/skills/qad/assets/nemotron-cascade-2-blend.yaml:
- Around line 5-43: Pin every nvidia/Nemotron-Cascade-2-SFT-Data entry in the
blend configuration to an immutable dataset revision using the schema’s
supported revision field. If the blend schema cannot express revisions, update
the generation flow to record and validate the resolved revision in the output
artifact, preserving the requirement from SKILL.md for an exact source revision.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8c080adc-5bf5-4c53-95fb-5a0e5b8a1bad

📥 Commits

Reviewing files that changed from the base of the PR and between 33d05b0 and 95e29fa.

📒 Files selected for processing (5)
  • .agents/skills/qad/SKILL.md
  • .agents/skills/qad/assets/nemotron-cascade-2-blend.yaml
  • .claude/skills/qad
  • examples/megatron_bridge/distill.py
  • tests/examples/megatron_bridge/test_qad.py

Comment on lines +160 to +164
Keep `train_iters=1000` from the first run so the cosine schedule has a stable
horizon. Set `save_interval=50`, `eval_interval=25`, `eval_iters=2`,
`exit_interval=150`, and `exit_duration_in_mins=220` for the initial stage. This
preserves checkpoints at iterations 50, 100, and 150. The duration exit is a
checkpointing safety margin, not the training target.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle duration exits before iteration 150 explicitly.

exit_duration_in_mins can save and exit before exit_interval=150. The following instructions nevertheless require checkpoints 50, 100, and 150 and direct the operator to export/evaluate “QAD-150”, which can cause a missing-checkpoint failure or mislabel an earlier checkpoint. Record the actual exit iteration; run the 150-step gate only when checkpoint 150 exists, otherwise mark the stage incomplete and resume or relaunch.

Also applies to: 184-190

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/qad/SKILL.md around lines 160 - 164, Update the initial-stage
instructions around the training configuration and the QAD-150 export/evaluation
steps to record the actual exit iteration, distinguish duration-triggered exits
before iteration 150, and run the 150-step gate only when checkpoint 150 exists;
otherwise mark the stage incomplete and direct the operator to resume or
relaunch instead of labeling an earlier checkpoint as QAD-150.

Comment on lines +164 to +184
parser.add_argument("--eval_interval", type=int, default=100, help="Validate every <N> steps")
parser.add_argument(
"--eval_interval", type=int, default=100, help="Validate + checkpoint every <N> steps"
"--eval_iters", type=int, default=32, help="Number of batches per validation stage"
)
parser.add_argument(
"--eval_iters", type=int, default=32, help="Number of batches per validation stage"
"--save_interval",
type=int,
default=None,
help="Checkpoint every <N> steps; defaults to --eval_interval",
)
parser.add_argument(
"--exit_interval",
type=int,
default=None,
help="Save a checkpoint and exit when the iteration is divisible by this value",
)
parser.add_argument(
"--exit_duration_in_mins",
type=int,
default=None,
help="Save a checkpoint and exit after this many minutes",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file around the referenced lines.
sed -n '140,230p' examples/megatron_bridge/distill.py

# Find where these args are consumed.
rg -n "eval_interval|eval_iters|save_interval|exit_interval|exit_duration_in_mins" examples/megatron_bridge/distill.py

Repository: NVIDIA/Model-Optimizer

Length of output: 4465


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for validation patterns and relevant config wiring.
rg -n "max\(.*0|assert .*interval|if .*interval|interval .*>=|>= 0|> 0|validate.*interval|check.*interval" examples/megatron_bridge/distill.py

Repository: NVIDIA/Model-Optimizer

Length of output: 441


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- distill.py excerpt ---'
sed -n '1,260p' examples/megatron_bridge/distill.py

echo
echo '--- interval references ---'
rg -n "eval_interval|eval_iters|save_interval|exit_interval|exit_duration_in_mins" examples/megatron_bridge/distill.py

echo
echo '--- validation patterns ---'
rg -n "max\(.*0|assert .*interval|if .*interval|interval .*>=|>= 0|> 0|validate.*interval|check.*interval" examples/megatron_bridge/distill.py

Repository: NVIDIA/Model-Optimizer

Length of output: 11985


🏁 Script executed:

#!/bin/bash
set -euo pipefail
nl -ba examples/megatron_bridge/distill.py | sed -n '150,240p'

Repository: NVIDIA/Model-Optimizer

Length of output: 200


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for other validation or semantic handling of the interval fields in the repo.
rg -n "save_interval|exit_interval|exit_duration_in_mins|eval_interval|eval_iters" .

# Inspect the distillation config wiring where these args are passed through.
sed -n '340,420p' examples/megatron_bridge/distill.py

Repository: NVIDIA/Model-Optimizer

Length of output: 8113


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for these names in the repository, but keep the output compact.
rg -n "save_interval|exit_interval|exit_duration_in_mins|eval_interval|eval_iters" examples megatron_bridge . -g '!**/.git/**' -g '!**/node_modules/**' || true

echo
echo '--- wiring excerpt ---'
sed -n '340,420p' examples/megatron_bridge/distill.py

Repository: NVIDIA/Model-Optimizer

Length of output: 11754


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- wiring excerpt ---'
sed -n '340,420p' examples/megatron_bridge/distill.py | cat -n

Repository: NVIDIA/Model-Optimizer

Length of output: 3825


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find any local handling or documentation of the interval flags.
rg -n "save_interval|exit_interval|exit_duration_in_mins|eval_interval|eval_iters" examples/megatron_bridge

Repository: NVIDIA/Model-Optimizer

Length of output: 2564


Validate the interval args at the CLI boundary. --eval_interval and --eval_iters are only checked under --validate_only, and --save_interval, --exit_interval, and --exit_duration_in_mins are passed through unchecked. Reject zero/negative values explicitly, or document a sentinel if 0 is meant to disable scheduling.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/megatron_bridge/distill.py` around lines 164 - 184, Validate
eval_interval, eval_iters, save_interval, exit_interval, and
exit_duration_in_mins immediately after CLI parsing, rejecting zero or negative
values before they reach scheduling or validation logic; if any zero value is
intentionally supported as a disable sentinel, handle it explicitly and
consistently instead.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant