[Cherry-pick] PRs #2022 #2026 #2032 #1981 #2010 #2043 #2042 #2038 #2050 #2041 #2031 #2061 #1983 #1628 - #2062
Conversation
…2022) ### What does this PR do? Type of change: Documentation update - Update documentation guide for ONNX INT4 PTQ on Windows cuda13 host - mention about compatible onnxruntim-gpu and cupy-cuda13x packages. ### Testing - Windows's onnx_ptq\genai_llm INT4 PTQ example with a 1B genai-cuda-ep ONNX model + local doc building ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: ✅ / ❌ / N/A <!--- If ❌, explain why. --> - If you copied code from any other sources or added a new PIP dependency, did you follow guidance in `CONTRIBUTING.md`: ✅ / ❌ / N/A <!--- Mandatory --> - Did you write any new necessary tests?: ✅ / ❌ / N/A <!--- Mandatory for new features or examples. --> - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ / ❌ / N/A <!--- Only for new features, API changes, critical bug fixes or backward incompatible changes. --> - Did you get Claude approval on this PR?: ✅ / ❌ / N/A <!--- Run `/claude review`. NVIDIA org members can self-trigger for complex changes; orthogonal to CodeRabbit. --> ### Additional Information <!-- E.g. related issue. --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified Windows CUDA prerequisites for calibration and GPU-accelerated quantization. * Added setup guidance for CUDA 12 and CUDA 13.x, including compatible packages and cuDNN requirements. * Expanded installation verification steps for CUDA, ONNX Runtime, and CuPy. * Updated the GenAI LLM example with CUDA version compatibility guidance. * **Enhancements** * Added runtime logging of detected CUDA environment paths and version details during quantization. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: vipandya <vipandya@nvidia.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do? **Type of change:** Bug fix (CI) `mcp==2.0.0` was released and removed the `mcp.server.fastmcp` module (the 2.0 SDK replaces it with `mcp.server.mcpserver`). `tools/mcp/pyproject.toml` declared an unpinned `mcp>=1.0`, so CI now resolves `mcp==2.0.0`, and `tools/mcp/modelopt_mcp/server.py`'s `from mcp.server.fastmcp import FastMCP` fails at import: ``` ModuleNotFoundError: No module named 'mcp.server.fastmcp' ERROR collecting tools/mcp/tests/test_bridge.py ``` This breaks the `mcp` unit job — and thus the `unit-pr-required-check` gate — on **every** PR whose diff touches `pyproject.toml`, `noxfile.py`, or `.github/workflows/unit_tests.yml` (the changed-files paths that trigger the `mcp` job). This PR pins `mcp>=1.0,<2`, keeping the 1.x line that still ships `mcp.server.fastmcp`. Migrating the server to the mcp 2.0 API (`mcp.server.mcpserver`) is a larger change tracked separately. ### Usage ```python # N/A - dependency pin only ``` ### Testing - `uv pip install -e tools/mcp` now resolves an `mcp` 1.x wheel, so `from mcp.server.fastmcp import FastMCP` imports and `tools/mcp/tests/test_bridge.py` collects again. ### 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 — tightens an existing dependency's upper bound. - Did you write any new necessary tests?: N/A — restores collection of the existing `tools/mcp` tests. - Did you update Changelog?: N/A — CI/build fix, nothing user-facing in the wheel. - Did you get Claude approval on this PR?: ❌ ### Additional Information Unblocks the `unit-pr-required-check` gate for in-flight PRs (surfaced on #2000). Follow-up: migrate `modelopt_mcp/server.py` to the mcp 2.0 API and relax the pin. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Prevented compatibility issues by limiting the MCP package to supported version 1.x releases. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Fridah-nv <201670829+Fridah-nv@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do? Type of change: Bug fix Prevents recursively collected text-submodel reverse mappings from rewriting an already nested multimodal model namespace during unified Hugging Face export. Transformers reverses the Qwen3.5 text mapping into a broad `^model.` -> `model.language_model.` rename. ModelOpt previously applied that rule to every key in the full VLM state dict, moving `model.visual.*` under the language model and nesting `model.language_model.*` twice. This change drops the reverse rule only when its target child namespace is already registered. Standalone text models continue to use the conversion. ### Usage ```python # Existing export_hf_checkpoint usage is unchanged. ``` ### Testing - `python -m pytest -q tests/unit/torch/export` (`112 passed`, `1 skipped` because optional Diffusers is not installed) - Targeted pre-fix reproduction confirmed both malformed Qwen3.5 namespaces; both regression cases pass after the fix - Tiny `Qwen3_5MoeForConditionalGeneration` meta-device model-tree probe passed - `python -m pre_commit run --files modelopt/torch/export/quant_aware_conversion.py tests/unit/torch/export/test_quant_aware_conversion.py` ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ - Did you get Claude approval on this PR?: N/A ### Additional Information No API or dependency changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved model conversion to prevent incorrect renaming of nested visual or sibling model components. * Fixed duplicate namespace prefixes in converted model weights. * Preserved correct reverse mapping for text-only model configurations. * **Tests** * Added coverage for nested multimodal and text-only model conversion scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: weimingc <17592131+meenchen@users.noreply.github.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
… and HF embedding ONNX export example (#1981) ### What does this PR do? Type of change: new example Without output-side quantization, TensorRT's quantized GEMMs emit FP16 activations: every quantized GEMM input adds a low-precision copy *on top of* the FP16 tensors instead of replacing them, so FP8/FP4 engines can use as much or more activation memory than an unquantized FP16 engine ([5726458]). Quantizing the projection-Linear outputs makes the engine carry inter-layer activations in the low-precision format. This PR ships that as recipes for the Llama-Nemotron embedding/reranking family (NVFP4 and FP8 variants) plus an end-to-end example. Measured on RTX PRO 6000 Blackwell with TensorRT 10.16 (strongly-typed engines, 5 dynamic-shape profiles up to 32x512), activation memory per profile: | Model | FP16 | `fp8` preset | **fp8 recipe** | `nvfp4` preset | **nvfp4 recipe** | |-------|-----:|-------------:|---------------:|---------------:|-----------------:| | llama-nemotron-embed-1b-v2 | 1040 MiB | 1392 MiB | **1096 MiB** | 1040 MiB | **516 MiB** | | llama-nemotron-rerank-1b-v2 | 1040 MiB | 1392 MiB | **1096 MiB** | 520 MiB | **331 MiB** | Engine sizes (dominated by weights): FP16 ≈ 2374 MiB, FP8 ≈ 1453 MiB, NVFP4 ≈ 1050–1075 MiB. The presets only shrink weights — their activation memory matches (or exceeds, for FP8) the FP16 engine because every quantized GEMM still emits FP16; the output-quantizer recipes are what reduce activation memory (fp8: −21% vs its preset; nvfp4: −50% vs its preset and 2x below FP16). - **`modelopt_recipes/huggingface/nemotron_llama/ptq/nvfp4_output_quant_proj.yaml`** — the general `nvfp4` preset plus dynamic NVFP4 output quantizers scoped to the projection Linears (`*_proj.output_quantizer`). Scoping matters: a `DynamicQuantize` on non-GEMM outputs (embedding lookup, pooling) fails to compile in TensorRT. The sequence-classification `score` head is kept unquantized: final heads stay in high precision like `lm_head`, and its `[1, hidden]` weight cannot be packed by the NVFP4 exporter. - **`modelopt_recipes/huggingface/nemotron_llama/ptq/fp8_output_quant_proj.yaml`** — the FP8 twin: per-tensor FP8 output quantizers on the projection Linears, switching the engine from FP16-out GEMMs (`e4m3f16..._bias_f16`) to FP8-out GEMMs (`e4m3e4m3_e4m3`). - **`examples/torch_onnx/hf_embedding_quant_to_onnx.py`** — minimal end-to-end recipe-driven quantize → ONNX export for HF bidirectional Llama embedding and reranking encoders (auto-detected from the model architecture; embedding models export mean-pooled L2-normalized embeddings, rerankers export relevance logits), with the export shims needed for a TensorRT-fusable graph (bidirectional sdpa symbolic with single-precision attention constants; static blocked-axis extents for `Reshape → TRT_FP4DynamicQuantize`). - **`modelopt/torch/quantization/export_onnx.py`** — `configure_linear_module_onnx_quantizers` now types output quantizers as `"dynamic"` (they previously fell to the static path, which the NVFP4 weight exporter rejects on activations), and the sdpa symbolic's `JitScalarType` import is fixed for torch >= 2.11. - **`examples/torch_onnx/torch_quant_to_onnx.py`** — replaces the `mtq.*_CFG` module-constant table with YAML recipe loading and adds a `--recipe` flag (preset basename or `QuantizeConfig` YAML path); `--auto_quantization_formats` values switch from config-constant names to preset basenames. - **`tests/examples/torch_onnx/test_hf_embedding_quant_to_onnx.py`** — end-to-end example test running both model kinds through quantize → export with tiny random-weight stand-ins (plain Llama encoder and `LlamaForSequenceClassification`, sized to the NVFP4 block size). - README section for the new example, including the results table and trtexec engine-build steps; CHANGELOG entry. ### Usage ```bash # Embedding model (default recipe: nvfp4 + projection output quantizers) python examples/torch_onnx/hf_embedding_quant_to_onnx.py \ --model_path=nvidia/llama-nemotron-embed-1b-v2 \ --trust_remote_code \ --onnx_save_path=llama_nemotron_embed_nvfp4.onnx # Reranking model (auto-detected), FP8 variant of the recipe python examples/torch_onnx/hf_embedding_quant_to_onnx.py \ --model_path=nvidia/llama-nemotron-rerank-1b-v2 \ --trust_remote_code \ --recipe=huggingface/nemotron_llama/ptq/fp8_output_quant_proj \ --onnx_save_path=llama_nemotron_rerank_fp8.onnx # Build a strongly-typed TensorRT engine (Blackwell, TensorRT >= 10.11) trtexec --onnx=llama_nemotron_embed_nvfp4.onnx --stronglyTyped \ --saveEngine=llama_nemotron_embed_nvfp4.plan \ --minShapes=input_ids:1x2,attention_mask:1x2 \ --optShapes=input_ids:32x128,attention_mask:32x128 \ --maxShapes=input_ids:32x512,attention_mask:32x512 ``` ### Testing - New end-to-end example test `test_hf_embedding_quant_to_onnx.py` runs both model kinds (embedding + reranking) through quantize → ONNX export on tiny random-weight checkpoints; `test_torch_onnx_recipe_flag` covers the `--recipe` flag. - Ran the example end-to-end on GPU for both real models and both recipes; each graph parses with TensorRT strongly-typed mode (NVFP4 graphs carry 112 input-side + 112 output-side `TRT_FP4DynamicQuantize`; FP8 graphs carry the matching static Q/DQ placement). - Built strongly-typed 5-profile engines on RTX PRO 6000 Blackwell with TensorRT 10.16 for FP16 and both presets/recipes on both models; compared per-profile activation memory via `ICudaEngine.get_device_memory_size_for_profile_v2` (table above) and verified kernel selection (FP8 recipe → `e4m3e4m3_e4m3` FP8-out GEMMs; NVFP4 → block-scaled GEMMs). - Recipes pass `tools/precommit/check_modelopt_recipes.py`; `pre-commit` green on all changed files. ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - Is this change backward compatible?: ❌ — `torch_quant_to_onnx.py`'s `--auto_quantization_formats` values are renamed from config-constant names (e.g. `NVFP4_AWQ_LITE_CFG`) to preset basenames (e.g. `nvfp4_awq_lite`); the loaded configs are identical. Core APIs are backward compatible (the output-quantizer export typing is additive). - 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?: ✅ (`test_hf_embedding_quant_to_onnx` for both model kinds, `test_torch_onnx_recipe_flag`) - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ ### Additional Information The target model family requires remote modeling code, so the usage examples opt in explicitly with `--trust_remote_code`. Accuracy parity (embedding quality / reranking scores) of the output-quantized recipes has not been evaluated yet and should be validated before recommending them as defaults. 🤖 Generated with [Claude Code](https://claude.com/claude-code) > 🤖 _Generated by Claude (AI agent)._ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added NVFP4 and FP8 projection-output PTQ recipes for Llama-Nemotron embedding/reranking. * Added an end-to-end Hugging Face “quantize-to-ONNX” CLI workflow for TensorRT export. * Enhanced Torch→ONNX quantization with YAML-driven recipes via a new `--recipe` flag and improved auto-quantization format handling. * **Bug Fixes** * Improved ONNX export and TensorRT DynamicQuantize compatibility for scaled dot-product attention and quantizer export behavior. * **Documentation** * Updated Torch→ONNX example docs and PTQ recipe guidance (including Nemotron Llama). * **Tests** * Strengthened ONNX graph checks for recipe and Hugging Face export flows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> > 🤖 _Generated by Codex (AI agent)._ --------- Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### 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
```text
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.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Updated the QAD skill documentation with a clear “Execute in this
order” workflow, including a revised default recovery training policy.
* Added a new `nemotron-cascade-2` dataset blend configuration with an
increased token budget.
* Enhanced the MeGatron Bridge distillation CLI with stricter interval
argument validation and support for configurable save-and-exit controls.
* **Documentation**
* Expanded Megatron Bridge README guidance for dataset preparation,
token-budget recalculation, and resume expectations.
* **Tests**
* Improved distillation and QAD tests to validate early-exit behavior
and checkpoint expectations.
* Added unit tests covering distillation CLI interval validation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Signed-off-by: Meng Xin <mxin@nvidia.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do?
Type of change: Bug fix
`get_dataset_samples` divides `num_samples` evenly across splits and
dumps the remainder on the last one. When `num_samples` is smaller than
the split count, most splits get a quota of `0` — but the old code still
opened them:
```python
dataset_splits = [_load_split(s) for s in splits] # all splits, eagerly
for dataset, n in zip(dataset_splits, num_per_split):
for i, sample in enumerate(dataset): # starts the iterator even when n == 0
if i >= n:
break
```
Merely starting a streamed split's iterator makes `datasets` fetch its
first record batch, so a zero-quota split costs a real download and
yields nothing.
For the `nemotron-post-training-v3` combo (14 samples over 7 datasets →
2 each) this is **24 split-opens that contribute zero samples**,
including 4 splits of `nvidia/Nemotron-Math-v2`, whose parquet shards
are ~12.6 GB each. Streaming those means reading a footer at byte ~12.6
GB and then pulling multi-MB row groups, all discarded.
This PR loads splits lazily and skips zero-quota ones. **Sample output
is unchanged** — the remainder still lands on the last split, so the
same rows are returned.
### Testing
`tests/gpu/torch/utils/test_dataset_utils.py::test_get_dataset_dataloader_nemotron_v3_chat_template`
has been timing out on the 120 s `tests/gpu` cap in CI since 2026-07-31
— on `main`'s nightly and on every PR that runs the GPU job
([example](https://github.com/NVIDIA/Model-Optimizer/actions/runs/30657911036/job/91249036652)).
The dumped stack shows it blocked in a socket read inside
`hf_file_system._fetch_range` while streaming `Nemotron-Math-v2`.
Reproduced locally against the real gated datasets and measured:
| | runtime | result |
|---|---|---|
| before | 120.00 s | **FAIL** — same `Timeout (>120.0s)` as CI |
| after | 25 s | **PASS** |
- `tests/gpu/torch/utils/test_dataset_utils.py` — 7 passed
- `tests/unit/torch/utils/test_dataset_utils.py` — 49 passed
Note the timeout is not a recent code regression: the test ran 50–72 s
for weeks, then stepped to >120 s on 2026-07-31 with no ModelOpt commit,
dependency change (`datasets` 4.8.5 / `huggingface_hub` 1.14.0 /
`hf-xet` 1.5.0 / `pyarrow` 24.0.0 identical across the boundary), or
dataset-repo commit to explain it — Hub-side latency on Xet-backed range
reads is the likely trigger. The wasted downloads were what left the
test with no margin to absorb it; it now has ~4× headroom.
### 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?: N/A — existing
`tests/gpu/torch/utils/test_dataset_utils.py` covers this path and is
the test the fix unblocks
- Did you update
[Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?:
N/A — internal perf fix, no API or behavior change
- Did you get Claude approval on this PR?: ❌ — not yet run
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Performance Improvements**
* Dataset splits are now loaded only when needed during sample
iteration.
* Splits with no assigned samples are skipped, reducing unnecessary
initialization.
* **Enhancements**
* Dataset loading supports specifying an optional split name.
* Sample quotas are calculated independently for each split.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
) ### What does this PR do? Type of change: Bug fix (agent skill documentation) The `evaluation` skill instructed the agent to **"append `-cu130` to the image tag"** for NVFP4 checkpoints on Blackwell B300/GB300 (sm_103). That was correct for v0.19.x, but **vLLM inverted its tag convention at v0.20.0**: the *unsuffixed* tag is now the CUDA-13 build, and `-cu129` is the CUDA-12 opt-out. Consequences of the stale rule: - `v0.20.1-cu130` / `v0.24.0-cu130` / `v0.26.0-cu130` **do not exist** — following the rule literally asks for a missing tag. - The documented fallback (`cu130-nightly-<arch>`) points at ~v0.20-era builds that are *older* than several models' documented minimum vLLM version, so it can't serve as an escape hatch either. This replaces the "append a suffix" instruction with a version-keyed table plus the durable check: **select a tag whose config blob reports `CUDA_VERSION` ≥ 13**, resolving the child manifest for the platform you actually deploy on. While the PR was open the default image was also bumped, and review surfaced two follow-on corrections. Full contents: 1. **Tag-convention fix** — version-keyed table, `-cu130` fallback removed. 2. **Default image `v0.19.1` → `v0.26.0`** (latest vLLM release) everywhere it was pinned: `SKILL.md` Step 3 and the Step 7.5 table, `example_eval.yaml`, `example_eval_next.yaml`. Version specifics that the bump made stale or self-contradictory were dropped (the `e.g. v0.20.0` bump example and the MiniMax-M2.7 `≥0.20.0` anecdote, both now *below* the default; the failure-mode lesson is kept). 3. **Convention boundary corrected to v0.20.0** — the first draft said `≤ v0.20.x` suffixed / `≥ ~v0.21` unsuffixed. Off by a minor release in both rows; see Testing. 4. **Config blob resolved per deployment platform** — the check said "arm64 child". GB300/Grace is arm64, but plenty of B300 deployments are `linux/amd64`. 5. **Same corrections applied to the `deployment` skill**, which carried the original append rule untouched: its NVFP4 note, `references/support-matrix.md`, `references/benchmarking.md`, and the `:latest` pins in `references/setup.md` (now `v0.26.0`, matching the evaluation skill's never-`:latest` stance). An earlier revision of this branch also carried a `.claude/skills/benchmark-model-kernels` symlink, added automatically by `tools/precommit/sync_claude_skills.sh` — it repairs missing symlinks repo-wide on any touch of `.agents/skills/`, and #1980 landed that skill without its link. It has been dropped from this branch to keep the scope on the vLLM image guidance. Worth its own one-line PR: without the symlink, Claude Code doesn't load that skill at all. ### Usage ```bash # Durable check — resolve the child manifest for YOUR platform (arm64 for # Grace/GB300, amd64 for x86) and read CUDA_VERSION from its config blob: # v0.19.1 -> CUDA_VERSION=12.9.1 (unsuffixed = CUDA 12, old convention) # v0.19.1-cu130 -> CUDA_VERSION=13.0.1 (suffixed = CUDA 13, old convention) # v0.20.0 -> CUDA_VERSION=13.0.2 (transition release: ships both suffixes) # v0.26.0 -> CUDA_VERSION=13.0.2 (unsuffixed = CUDA 13, new convention) # v0.26.0-cu129 -> CUDA_VERSION=12.9.1 (suffixed = CUDA 12, new convention) ``` ### Testing Verified empirically against the Docker registry API for `vllm/vllm-openai` — resolved each tag's child manifests and read `CUDA_VERSION` / `TORCH_CUDA_ARCH_LIST` from the config blob. **Where the convention flips (arm64):** | release | unsuffixed | `-cu130` | `-cu129` | |---|---|---|---| | v0.18.0 | 12.9.1 | 13.0.1 | absent | | v0.19.0 / v0.19.1 | 12.9.1 | 13.0.1 | absent | | **v0.20.0** | **13.0.2** | 13.0.2 | 12.9.1 | | v0.20.1 | 13.0.2 | **absent** | 12.9.1 | | v0.20.2 | 13.0.2 | **absent** | 12.9.1 | | v0.21.0 … v0.26.0 | 13.0.2 | absent | 12.9.1 | v0.20.0 is the transition release — it publishes both suffixes *and* its unsuffixed tag is already CUDA 13. That duplication is what hid the boundary: confirming `v0.20.0-cu130` exists reads as "old convention still applies at 0.20", while `v0.20.1-cu130` and `v0.20.2-cu130` don't exist at all. **Why the platform matters.** `CUDA_VERSION` is identical across children on every tag checked (v0.26.0, v0.26.0-cu129, v0.20.0, v0.19.1, v0.19.1-cu130, kimi-k3), but `TORCH_CUDA_ARCH_LIST` is not: ``` v0.26.0 amd64 7.5 8.0 8.6 8.9 9.0 10.0 12.0 v0.26.0 arm64 8.0 8.7 8.9 9.0 10.0 11.0 12.0 v0.19.1 amd64 7.0 7.5 8.0 8.9 9.0 10.0 12.0 v0.19.1 arm64 8.7 8.9 9.0 10.0+PTX 12.0 ``` `11.0` appears only on arm64, `7.5` / `8.6` only on amd64 — so the arch check has to read the child you'll actually run. **Default bump.** The `0.26.0` family is `{,-aarch64,-x86_64} × {,-cu129} × {,-ubuntu2404}` — 12 tags, `cu129` the only CUDA axis, no `-cu130`. The new default is therefore already a CUDA-13 build, and NVFP4 on B300/GB300 needs no suffix at all. Docs-only change; no runtime code touched. `pre-commit` clean. ### 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?: N/A (agent skill documentation) - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A - Did you get Claude approval on this PR?: ❌ (not yet run) ### Additional Information Split out of the GDPVal skill work (#2039) because it is independent of GDPVal and applies to every NVFP4-on-Blackwell deployment the skill generates. Now spans both the `evaluation` and `deployment` skills, all under `.agents/`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated deployment and evaluation guidance to use vLLM v0.26.0. * Clarified CUDA image-tag conventions, including CUDA 13 defaults and CUDA 12 opt-outs. * Added validation guidance for resolved CUDA versions, platform architecture settings, and image compatibility. * Updated NVFP4 Blackwell B300/GB300 support notes, including required `sm_103` kernel availability. * Refreshed example recipes, setup commands, benchmarking notes, and serving-image requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
…-low_memory_mode) (#2038) Type of change: Bug fix Fixes unified HF export of already-compressed NVFP4 weights — `mtq.compress` and, through it, `examples/llm_ptq/hf_ptq.py --low_memory_mode` (NVBug 5987078). Two defects, one root cause: the NVFP4 export branch has no handling for weights that were already real-quantized, unlike the `FP8_PB_REAL` branch which consumes `weight_quantizer._scale`. **1. `weight_scale` was recomputed from packed data.** After compression the weight is a `QTensorWrapper` of packed NVFP4 nibbles, and `QTensorWrapper.__new__` builds the Parameter from `_quantized_data`, so `.shape` reports the *packed* shape (the logical shape survives only in `metadata["shape"]`). The export derived the block count from `weight.shape[-1]` and took amax over nibble-pair bytes, so it wrote a scale of half the required size with meaningless values: | | `weight` | `weight_scale` written | expected | | --- | --- | --- | --- | | TinyLlama-1.1B `q_proj` | `[2048, 1024]` U8 | `[2048, 64]` | `[2048, 128]` | | DeepSeek-R1-Distill-Llama-70B `q_proj` | `[8192, 4096]` U8 | `[8192, 256]` | `[8192, 512]` | **2. An internal quantizer buffer leaked into the checkpoint.** `postprocess_state_dict` strips `weight_quantizer.<name>` for every name in `RealQuantLinear.list_of_scale_tensors`, but that list carried `"double_scale"` where the buffer is `_double_scale` — a missing underscore. So `_scale` was stripped and `_double_scale` was not, and it reached the checkpoint as `*.weight_quantizer._double_scale` (560 entries in the 70B checkpoint). Downstream loaders reject it before loading any weight: ``` KeyError: 'layers.0.mlp.down_proj.weight_quantizer._double_scale' RuntimeError: Engine core initialization failed. ``` This is why only the TensorRT backend appeared usable in the bug report — its converter tolerates the stray key, then produces `!!!!!!` output from the broken scales, while the PyTorch backend (vLLM / TensorRT-LLM) fails to load outright. The fix reuses the per-block scale captured at compression time, rescaled into the exported `weight_scale_2` convention, and corrects the typo above. The rescale matters: compression normalizes per-block FP8 scales against the global scale it captured at that moment, which is not the post-calibration `weight_scale_2` the export writes. Exporting the stored scale as-is loads fine but leaves every block off by a constant factor (~1.96x measured), so the `weight_scale * weight_scale_2` product that dequantization consumes must be preserved. Note the typo fix also affects `modelopt/torch/quantization/plugins/megatron.py:505,511`, which filter on the same list — these are internal buffers so excluding them looks correct, but calling it out since it is a behavior change outside the export path. **Compression-time scale layout.** `TensorQuantizer._real_quantize` calls `NVFP4QTensor.quantize(..., try_tensorrt=True)`, so on an FP4-capable device with TensorRT-LLM importable the stored `_scale` is the **cutlass-swizzled 1-D uint8** scale rather than the modelopt 2-D E4M3 layout. Confirmed on GB10 in a TRT-LLM container: ``` logical weight (512, 256) -> modelopt scale should be (512, 16) e4m3 _scale : (8192,) torch.uint8 (ndim=1) <- cutlass-swizzled after cutlass_fp4_scale_to_modelopt_fp4_scale: (512, 16) torch.float8_e4m3fn ``` The export therefore normalizes it the same way `NVFP4QTensor.dequantize` does, and raises if `tensorrt_llm` cannot be imported to convert, rather than writing raw byte values. No API change. The previously broken path now works: ```bash python hf_ptq.py --pyt_ckpt_path <local_ckpt_dir> --qformat nvfp4 \ --low_memory_mode --export_path <out> ``` All runs on DGX Spark (GB10, sm121, aarch64). Both compression-time scale layouts are covered, since the layout depends on whether TensorRT-LLM is importable in the process: **New tests** - `tests/gpu/torch/export/test_export_weight_gpu.py::test_export_compressed_nvfp4_weight` — dense E4M3 path. Asserts the per-block scale covers the logical input dim, that `weight_scale * weight_scale_2` matches an uncompressed export of the same model, and that `postprocess_state_dict` strips both internal buffers. - `tests/gpu_trtllm/torch/export/test_export_compressed_nvfp4.py::test_export_compressed_nvfp4_weight_trtllm_scale` — cutlass-swizzled path. Asserts as a *precondition* that the environment really produced a 1-D uint8 scale, so it cannot silently degrade into the dense case when TensorRT-LLM is absent. **Results** | suite | TensorRT-LLM 1.3.0rc17 container | vLLM 26.05 container | | --- | --- | --- | | both files above | 3 passed | 2 passed, 1 skipped | **Negative controls** (each test fails without the code it guards) - On `main` with only the test file applied: `test_export_compressed_nvfp4_weight` fails. - With only the un-swizzle conversion neutered, the rest of the fix intact: `test_export_compressed_nvfp4_weight_trtllm_scale` fails. **End-to-end, TensorRT-LLM container** (the environment from the bug report, where `_scale` is swizzled) — TinyLlama-1.1B, `--qformat nvfp4 --low_memory_mode`, plus a normal export as control. Exported checkpoints are structurally identical (0 stray `_double_scale` keys vs. 560 on `main`; 663 keys each; `q_proj.weight_scale` `[2048, 128]` FP8 in both; QKV `weight_scale_2` unified in both). Loaded on the **TensorRT-LLM PyTorch backend** — the backend reported as unusable: ``` GEN: France and is the most populous city in the country. It is located on the Seine River... GEN: France and is the most popular tourist destination in the country. It is a city of art, history... ``` On `main` that same load fails with `KeyError: '...weight_quantizer._double_scale'` before a single weight is read. **End-to-end, vLLM container** (dense scale path) — TinyLlama-1.1B, NVFP4 + `--low_memory_mode`: - before: `KeyError: '...weight_quantizer._double_scale'`, engine fails to start - after: loads and generates coherently (`"Paris is the capital of"` → `" France and is best known for the awe inspiring Notre-Dame C"`) - The fixed checkpoint is structurally equivalent to a normal (non-low-memory) export: identical key set (663 tensors), `input_scale` identical, and `weight_scale_2` identical — including the values unified across fused groups, so the fused-GEMM contract (one shared `weight_scale_2` per QKV / gate-up group) still holds. - DeepSeek-R1-Distill-Llama-70B (the model in the bug) reproduces the same signature on `main` and is the source of the numbers in the table above. full-memory PTQ This restores a working checkpoint, but `--low_memory_mode` NVFP4 is **not numerically identical** to a normal PTQ, and cannot be made so at export time. The nibbles are packed at *load* time against the layer's own global scale, so the effective per-block scale baked into them is `fp8_own * ws2_own`. `preprocess_linear_fusion` later unifies `weight_scale_2` across a fused group to the group max, and the format requires the per-block scale be E4M3, so the best the export can write is `round_fp8(fp8_own * ws2_own / ws2_unified)`. For the group member owning the max amax that ratio is exactly 1 and the round trip is bit-exact; for the others it costs one extra E4M3 rounding, bounded by a half-ULP (6.25%). The uncompressed path never pays this because its weights are still high precision at export, so `to_quantized_weight` re-quantizes the nibbles *after* unification. Measured on TinyLlama-1.1B (weight relative error vs. the source BF16 weights, 154 quantized tensors): | | mean rel. error | | --- | --- | | normal PTQ | 0.090248 | | `--low_memory_mode` (this PR) | 0.091443 | The degradation is confined to exactly 66 of 154 tensors = 22 layers x 3, i.e. the non-max members of each `q/k/v` and `gate/up` group (worst observed +0.0066, e.g. `layers.11.self_attn.k_proj` 0.0898 -> 0.0964). The 88 remaining tensors — group winners plus the unfused `o_proj` / `down_proj` — are bit-exact. Removing this requires compressing a fusion group against one shared scale in the compress-on-load path (`RealQuantParameterDict`), where the group max first becomes known; that is a larger change and is left as a follow-up. - 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ - Did you get Claude approval on this PR?: ❌ NVBug 5987078. Not addressed here: at 70B scale on DGX Spark, `--low_memory_mode` can also crash *before* export when the device map offloads, because `QTensorWrapper.to()` cannot represent a `meta` tensor: ``` accelerate/hooks.py: set_module_tensor_to_device(module, name, "meta") RuntimeError: Attempted to call `variable.set_data(tensor)`, but `variable` and `tensor` have incompatible tensor type. ``` It is reproducible on demand under low free GPU memory (crashes at 41.2 GB and 58.5 GB free; succeeds at 88.1 GB) and is easy to hit on Spark's unified memory, where page cache from reading the checkpoint counts against `torch.cuda.mem_get_info()`. That is an independent defect and will be filed separately. Signed-off-by: Chenjie Luo <chenjiel@nvidia.com> Signed-off-by: Chenjie Luo <108829653+cjluo-nv@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
Transformers Dependency bump - Drop 4.56 (still support 4.57 with
deprecation note) and extend to 5.14 (nemo:26.08 ships with this
version)
- CI tests now use transformers 5.14
- Manually ran `tests/{gpu_megatron,examples/megatron_bridge}` in
nemo:26.08.rc3
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
- Updated supported Transformers versions to 4.57 through 5.14.
- Qwen3-VL models are now available without version-based restrictions.
- Updated the changelog to reflect the new minimum Transformers version
and upcoming removal of Transformers 4.x support.
- Updated validation and test configurations for the supported
Transformers versions.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do? Type of change: Bug fix Fixes NVBug 6524370. `DiffusionGemma` ties weights between its encoder and decoder. `get_model` loads with `device_map="auto"` (`examples/hf_ptq/example_utils.py`), and `"auto"` is an alias for `"balanced"` — accelerate splits the model evenly across all visible GPUs by size, with no awareness of tied parameters. On multi-GPU it can place the two sides of a tied pair on different devices; the tie then cannot be honored and one side is left on the `meta` device. The pre-quantization preview in `pre_quantize` then reaches `(input_ids == self.config.image_token_id).any()` in `generation_diffusion_gemma.py` and fails: ``` RuntimeError: Tensor.item() cannot be called on meta tensors ``` This is multi-GPU-only by construction: with one visible GPU the balanced split is trivial, nothing is separated, and nothing lands on `meta`. This PR detects DiffusionGemma configs in `get_model` and selects `device_map="sequential"`, which fills one GPU before spilling to the next and so keeps tied modules together. It mirrors the existing per-model handling for `bart` and `t5`, where `device_map="auto"` similarly mis-shards tied encoder/decoder weights. Detection reads `model_type` and `architectures` from the config and ignores underscores, since the family is spelled `diffusion_gemma` in the Transformers module path and `DiffusionGemma` in the class name. ### Usage No API change. Previously this needed the flag passed manually: ```bash python hf_ptq.py --model <diffusion-gemma-ckpt> --recipe <recipe> \ --export_path <out> --trust_remote_code --use_seq_device_map ``` It is now selected automatically, and the model load logs: ``` Detected DiffusionGemma model. Using device_map='sequential'; the balanced 'auto' mapping can split its tied encoder/decoder weights across GPUs. ``` Passing `--use_seq_device_map` explicitly still works and is unaffected. ### Testing - Reproduced on 4x GB200 with `diffusiongemma-26B-A4B-it` and the `nvfp4_experts_only` recipe; `--use_seq_device_map` resolves the crash, confirming the device-mapping cause. - Validated on oci-hsg (4x GB200): with this patch and no CLI flag, `diffusiongemma-26B-A4B-it` loads correctly and the meta-tensor crash no longer reproduces. - `is_diffusion_gemma` checked against both config spellings, `architectures=None`, `architectures=[]`, and a `gemma3` negative to confirm no over-match — `get_model_type` already orders `DiffusionGemma` before `Gemma` for exactly this substring-collision reason. - `pre-commit run --files examples/hf_ptq/example_utils.py` passes (ruff, ruff-format, mypy, bandit). ### 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?: ❌ — no existing unit coverage for `get_model` device-map selection; happy to add a config-level test for `is_diffusion_gemma` if wanted. - Did you update [Changelog](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A - Did you get Claude approval on this PR?: ❌ — pending ### Additional Information NVBug 6524370. Same class of failure as the existing `t5` workaround in `get_model`; a general "any tied encoder/decoder model" rule was considered but rejected, since `tie_word_embeddings=True` holds for most decoder-only LLMs where `auto` is fine and forcing sequential would regress large-model runs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved DiffusionGemma model loading on multi-GPU systems by keeping related model weights together. * Added more reliable DiffusionGemma model recognition across supported configurations. * Preserved existing automatic device allocation for single-GPU systems and other supported models. * Improved loading reliability by applying appropriate memory limits during multi-GPU setup. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Juhi Mittal <juhim@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do? Type of change: new example + small launcher / modelopt-example features (backward compatible) Adds end-to-end ModelOpt **launcher** pipelines for the Megatron-Bridge flow on Nemotron-3-Nano-30B-A3B, the minimal launcher features to run them wrapper-free from YAML, and an **in-step accuracy gate** for Minitron pruning. **New launcher examples** (`tools/launcher/examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/`): - `mbridge_prune.yaml` — Minitron prune **with an in-step MMLU gate** → vLLM sanity gen (2 tasks) - `mbridge_quantize.yaml` — FP8 quantize → unified-HF export → MMLU gate on the vLLM backend, which doubles as the deploy sanity check (3 tasks). Matches the [tutorial](https://github.com/NVIDIA/Model-Optimizer/blob/main/examples/megatron_bridge/tutorials/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/README.md). **Prune accuracy gate** (reuses the search's own score — no separate eval step): - `modelopt/torch/prune/plugins/mcore_minitron.py` — `MCoreMinitronSearcher` now stores the exported best `CandidateSubnet` under `state_dict["best"]` (additive; sits beside the existing `sorted_layers` key). - `examples/megatron_bridge/prune_minitron.py` — new `--score_lower_bound`: reads `pruning_scores["best"].score` and exits non-zero if the exported model is below the floor. Score-agnostic (any `--prune_score_func`); rejected with `--prune_export_config` (manual pruning has no score). **Launcher (`tools/launcher`)** — run single-node Megatron-Bridge one-liners directly from YAML: - `SandboxTask.inline` — a command in the YAML, no `common/**/*.sh` wrapper (single-line; folded scalar) - `SandboxTask.reqs` / `reqs_file` — pip-install deps in the container before the command (shell-safe; on Slurm the install is rank-0-guarded so multi-rank tasks don't race) - `SlurmConfig.docker_user` — local-Docker user (e.g. `root`); ignored on Slurm - `get_default_env` honors `HF_HOME` / `TRITON_CACHE_DIR` env overrides, so a non-CI user can point caches at a writable path (the shared `/cicd/hf-cache` is owned by the CI account) - reject `args` together with `inline` **`examples/llm_eval/lm_eval_hf.py`**: - `--accuracy_lower_bound` — gate on the single requested task's `acc` (used by the quantize MMLU step; exits non-zero if below) - drop ModelOpt (hf-only) args for non-`hf` backends, so `--model vllm` works on a deployable quantized checkpoint ### Usage ```bash cd tools/launcher # Prune (in-step MMLU gate) -> vLLM gen uv run launch.py --yaml examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_prune.yaml --yes # FP8 quantize -> unified-HF export -> MMLU gate (vLLM) uv run launch.py --yaml examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_quantize.yaml --yes ``` ### Testing - Launcher unit tests for `inline`, `reqs`/`reqs_file`, `docker_user`, the `args`+`inline` guard, and example-resolve; ruff / mypy / bandit clean. `tests/examples/megatron_bridge/test_prune_minitron.py` now passes `--score_lower_bound=0.01` to exercise the gate path on the tiny models. - **End-to-end on the real Nemotron-3-Nano-30B-A3B (4×B200, OCI-HSG):** - Prune 30B → 3B-active: `[score_gate] mmlu_10pct = 0.5196 >= 0.45 PASS`; vLLM gen coherent. - FP8 quantize → unified-HF export (`Detected ModelOpt fp8 checkpoint`) → MMLU on the vLLM backend `acc = 0.7077 >= 0.60 PASS`. - Earlier smoke on **Qwen3-0.6B** in `nemo:26.06` through the same flow. ### Before your PR is "*Ready for review*" - Is this change backward compatible?: ✅ (new state-dict key is additive; new CLI args default to off) - 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: N/A <!-- tooling/examples + additive searcher state key --> - Did you get Claude approval on this PR?: ❌ <!-- pending --> ### Additional Information - **Container pinning:** saving a pruned Nemotron-H to HF requires `transformers<5`, so `mbridge_prune.yaml` runs on `nemo:26.04` (26.06 drops it); quantize/export run on `nemo:26.06`. - **`docker_user: root`** is set on all example tasks — local-Docker only (ignored on Slurm), needed so downstream tasks can read task_0's root-owned checkpoints and to read the image's root-only `/opt/Megatron-Bridge`. - The quantize MMLU step passes `enforce_eager=True` to vLLM — for a run-once eval this skips ~17 min of CUDA-graph capture / `torch.compile` with no accuracy change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added inline shell commands and per-task Python dependency installation for launcher workflows. - Added configurable Docker user selection and preservation of existing cache environment settings. - Added evaluation accuracy and pruning score gates that fail workflows below configured thresholds. - Added NVIDIA Nemotron pruning and quantization workflow examples. - Improved backend-specific handling of ModelOpt options. - **Documentation** - Documented inline commands, dependencies, variable substitution, and configuration examples. - **Bug Fixes** - Strengthened task execution validation and configuration checks. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
… Pipe symbols (#2061) Saving pruned Nemotron-3-Nano (with MTP) to HF format raised an assertion which is fixed here Tested on nemo:26.04 with transformers 4.57 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved exported hybrid layer patterns for pruned models by removing MTP and pipeline-parallel markers. * Ensured exported configurations accurately represent the model’s main layers. * **Tests** * Added coverage for pruning models with an MTP prediction layer and hybrid override patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
### What does this PR do? Type of change: Bug fix - Synchronizes existing ONNX tensor declarations when folding `Constant -> Cast` patterns, so the published `value_info` dtype matches the converted Constant payload. - Computes `GatherND` output shapes directly when autocast is running in custom-op mode, avoiding a generic input-0 shape copy for this shape-changing standard ONNX operator. - Restricts the previous input-0 shape fallback to actual custom-op outputs. - Adds regression tests for folded Constant dtype metadata and custom-op-mode `GatherND` shape propagation. ### Usage ```python # No user-facing API change. Existing Autocast usage remains: $ python -m modelopt.onnx.autocast --onnx=model.onnx ``` ### Testing - Reproduced the metadata issue with ModelOpt `0.44.0` and with main ToT `cba8a5c62a1a54fe89fb69bfd484ea0a653c633a` before the fix. - Verified the standalone reproduction no longer reports stale Constant dtype metadata or input-derived `GatherND` output shape after the fix. - Ran `pytest tests/unit/onnx/autocast/test_precisionconverter.py::test_folded_constant_cast_updates_value_info_type tests/unit/onnx/autocast/test_precisionconverter.py::test_custom_op_mode_uses_schema_shape_for_standard_gathernd -q` - Ran `pytest tests/unit/onnx/autocast/test_precisionconverter.py -q` ### Before your PR is "*Ready for review*" Make sure you read and follow [Contributor guidelines](https://github.com/NVIDIA/Model-Optimizer/blob/main/CONTRIBUTING.md) and your commits are signed (`git commit -s -S`). Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contributors) (e.g. avoiding hardcoded `trust_remote_code=True`, `torch.load(..., weights_only=False)`, `pickle`, etc.). - 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ❌ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Precision conversion now better preserves/restores full model input/output metadata when enabled, including after custom-op type/shape propagation. * Improved shape inference for standard ops during custom-op mode (including `GatherND`, `Gather`, `Unsqueeze`, and `Shape`), with correct scalar (rank-0) handling and stricter input-shape propagation. * When folding redundant `Cast` after `Constant`, element-type metadata is now updated consistently across the graph and nested subgraphs. * **Tests** * Added regression and custom-op mode tests for `Constant -> Cast -> Identity` folding and for `GatherND`/rank-change shape propagation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Gwena Cunha <4861122+gcunhase@users.noreply.github.com> Signed-off-by: Gwenaelle Cunha Sergio <gcunhasergio@nvidia.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
…) subgraphs during FP16/BF16 conversion (#1628) Type of change: Bug fix Fixes [6058841] — `python -m modelopt.onnx.quantization --high_precision_dtype fp16` crashed with *"Inconsistent type on If node"* on models containing control-flow `If`/`Loop`/`Scan` subgraphs. **Root cause.** The FP16/BF16 `PrecisionConverter` blindly converted *every* subgraph initializer to the parent control-flow node's precision, without the activation-bracketing casts it uses in the main graph. This left inconsistent tensor types that crash ONNX shape inference / TensorRT strongly-typed parsing: - A `Gemm` inside an `If` branch reading an outer-scope activation (fp32) ended up with fp16 weights → `B has inconsistent type tensor(float16)`. - `Resize` `scales` (which must stay fp32 per the ONNX spec) was converted to fp16 → `ParseData type mismatch ... Expected:float Actual:float16`. **Fix.** - A subgraph node is converted to low precision **only when all of its float inputs are subgraph initializers** eligible for low precision. Any node consuming a float activation / outer-scope tensor, or an input that must stay high precision (e.g. `Resize` `scales`), stays high precision — so each node's inputs share a single precision. - Float **outer-scope captures** and **low→high precision boundaries** inside subgraphs are reconciled with `Cast` nodes; a captured tensor's preserved subgraph `value_info` is synced to its real main-graph precision; control-flow node **outputs** are treated as high precision (a low-precision `If` branch may still convert eligible nodes *inside* its body, but the parent control-flow node's outputs stay high precision). - `Constant`→`Cast` folding in `remove_redundant_casts` now refreshes the constant's `value_info` so a same-type-constrained consumer (e.g. `Greater`) isn't left with a stale, conflicting type. (Pre-existing main-graph bug surfaced once the `If` models completed conversion.) **Known limitation.** A low-precision `Loop`/`Scan` whose body carries a **float loop-carried or scan-input state variable** is not yet reconciled here — its body's formal inputs are treated as high precision while the main graph casts the parent's inputs to low precision. This PR fully covers `If` branches (both precisions) and high-precision `Loop`/`Scan` bodies (outer-scope captures reconciled); the low-precision `Loop`/`Scan` body case is tracked as a follow-up. ```bash python -m modelopt.onnx.quantization \ --quantize_mode int8 --high_precision_dtype fp16 \ --onnx_path model.onnx \ --output_path model_strongType_int8+fp16.onnx ``` Validated on both reported models: | graph pattern | convert | strict `infer_shapes(check_type=True)` | ORT load | numerics vs FP32 | |---|---|---|---|---| | `If` branch with `Gemm` reading an outer-scope input | ✅ | ✅ | ✅ | bit-exact (Gemms kept fp32) | | `If` branch with `Resize` (`scales` initializer) | ✅ | ✅ | ✅ | max abs err 9e-5 (fp16 tol) | Also verified with `keep_io_types=False`. Added 5 regression tests in `tests/unit/onnx/autocast/test_precisionconverter.py` — `If` Gemm-with-outer-scope-input, `If` Resize-scales-stay-FP32, chained-`If` capture, high-precision `Loop`-body capture, and `Constant`→`Cast` fold `value_info` refresh — that fail without the fix and pass with it. Full `tests/unit/onnx/autocast/` suite passes (214). `pre-commit` clean. - 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ - Did you get Claude approval on this PR?: ❌ (draft) Fixes bug [6058841]. Draft pending final review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> - **Bug Fixes** - Fixed FP16/BF16 conversion for ONNX models with control-flow subgraphs (for example `If`/`Loop`), especially when `--high_precision_dtype fp16` is used. - Preserved FP32 precision for branch weights that depend on outer-scope FP32 activations. - Added/adjusted casts for precision reconciliation across subgraph boundaries and updated value type metadata after `Constant`→`Cast` folding. - **Tests** - Added regression tests covering control-flow nesting/chaining, `Resize` inputs that must remain FP32, initializer precision selection, and strict shape/type inference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: ajrasane <131806219+ajrasane@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
📝 WalkthroughWalkthroughThe PR adds QAD and launcher workflows, recipe-driven ONNX quantization, expanded ONNX autocast handling, deployment guidance updates, evaluation gates, export fixes, dataset-loading changes, and Transformers compatibility updates. ChangesDeployment guidance
QAD and Megatron workflows
Launcher execution workflows
ONNX quantization and autocast
ModelOpt runtime and export updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/0.46.0 #2062 +/- ##
===================================================
+ Coverage 66.83% 77.99% +11.15%
===================================================
Files 519 519
Lines 58916 59278 +362
===================================================
+ Hits 39376 46233 +6857
+ Misses 19540 13045 -6495
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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.
Actionable comments posted: 12
🧹 Nitpick comments (5)
tests/examples/megatron_bridge/test_prune_minitron.py (2)
43-51: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the MTP and hybrid-pattern output contract.
The fixture enables
num_nextn_predict_layers=1andmtp_hybrid_override_pattern, but the assertions only check output existence and parameter counts. They do not verify that MTP fields are removed or that the normalized hybrid pattern is persisted.Add direct assertions against the generated configuration or checkpoint metadata. Use the appropriate inspection path for Megatron and Hugging Face outputs.
As per path instructions, tests must exercise the behavior they claim to validate.
🤖 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 `@tests/examples/megatron_bridge/test_prune_minitron.py` around lines 43 - 51, Extend the test case using num_nextn_predict_layers=1 and mtp_hybrid_override_pattern to inspect both generated output formats: assert the Megatron configuration or checkpoint metadata has MTP fields removed, and assert the Hugging Face metadata persists the normalized hybrid pattern. Use each format’s existing inspection/loading helpers rather than relying only on output existence or parameter counts.Source: Path instructions
79-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the failing score-gate branch.
Both tests use
score_lower_bound=0.0, so they cover only the passing path. Add a case with a bound above the measured score and assert that the pruning command exits non-zero.As per path instructions, focused tests must protect the changed behavior and use the real implementation where applicable.
Also applies to: 148-148
🤖 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 `@tests/examples/megatron_bridge/test_prune_minitron.py` at line 79, Add a focused test case in the existing pruning tests around the score_lower_bound configuration, using a bound higher than the measured score and the real pruning command implementation. Assert that this failing score-gate path exits non-zero, while preserving the existing passing-path coverage.Source: Path instructions
tests/examples/megatron_bridge/test_qad.py (1)
113-124: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest duration-based exit independently.
This test always exits at iteration 2 through
exit_interval. The ten-minuteexit_duration_in_minsvalue cannot trigger. A broken duration-exit mapping could still pass this test.Add a focused configuration test or a separate integration case with
exit_intervalunset and a controlled duration-based exit.As per path instructions, tests must exercise the behavior they claim to validate.
🤖 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 `@tests/examples/megatron_bridge/test_qad.py` around lines 113 - 124, Update the test around the distillation command and tracker assertions to validate duration-based exit independently: add a focused configuration test or separate integration case with exit_interval unset, use a controlled exit_duration_in_mins value, and retain assertions confirming the run exits at the expected iteration/checkpoint. Ensure the existing interval-based test remains scoped to interval exit rather than claiming duration coverage.Source: Path instructions
modelopt/onnx/autocast/precisionconverter.py (1)
1620-1665: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReuse the existing maps instead of scanning the graph containers.
_get_tensor_shapeand_get_tensor_elem_typescanself.model.graph.initializerand the three value-info containers on every lookup._refresh_gathernd_pre_cast_declarationsalready rebuildsself.initializer_mapandself.value_info_mapimmediately before calling them, and both maps cover exactly these tensors.utils.setup_mappingspopulatesvalue_info_mapfromvalue_info,input, andoutput.Reading the maps keeps the behavior identical and removes the repeated linear scans over initializers, which matter on large models.
♻️ Proposed change to use the maintained maps
def _get_tensor_shape(self, tensor_name: str) -> list[int | str | None] | None: - initializer = next( - (value for value in self.model.graph.initializer if value.name == tensor_name), None - ) + initializer = self.initializer_map.get(tensor_name) if initializer is not None: return list(initializer.dims) - for value_info in ( - *self.model.graph.input, - *self.model.graph.output, - *self.model.graph.value_info, - ): - if value_info.name != tensor_name: - continue + value_info = self.value_info_map.get(tensor_name) + if value_info is not None: tensor_type = value_info.type.tensor_type - if not tensor_type.HasField("shape"): - continue - shape = [] - for dim in tensor_type.shape.dim: - if dim.HasField("dim_value"): - shape.append(dim.dim_value) - elif dim.HasField("dim_param"): - shape.append(dim.dim_param) - else: - shape.append(None) - return shape + if tensor_type.HasField("shape"): + shape = [] + for dim in tensor_type.shape.dim: + if dim.HasField("dim_value"): + shape.append(dim.dim_value) + elif dim.HasField("dim_param"): + shape.append(dim.dim_param) + else: + shape.append(None) + return shape🤖 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 `@modelopt/onnx/autocast/precisionconverter.py` around lines 1620 - 1665, Update _get_tensor_shape and _get_tensor_elem_type to resolve initializers through self.initializer_map and value metadata through self.value_info_map instead of scanning graph.initializer, input, output, and value_info. Preserve the existing shape, element-type, Cast-producer fallback, and missing-value behavior while relying on the maps rebuilt by _refresh_gathernd_pre_cast_declarations.docs/source/getting_started/windows/_installation_standalone.rst (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix inconsistent capitalization of "cuDNN".
Line 16 uses "CuDNN". Line 71 uses "cudnn". Lines 54 and 87 use "cuDNN". Use "cuDNN" consistently throughout the document.
Proposed fix
- - CUDA Toolkit and matching CuDNN for using CUDA path during calibration (e.g. for calibration of ONNX models using `onnxruntime-gpu` or CUDA EP) + - CUDA Toolkit and matching cuDNN for using CUDA path during calibration (e.g. for calibration of ONNX models using `onnxruntime-gpu` or CUDA EP)-The steps below assume a CUDA 13.x Toolkit, compatible cudnn, and a compatible driver are already installed on the host. +The steps below assume a CUDA 13.x Toolkit, compatible cuDNN, and a compatible driver are already installed on the host.Also applies to: 71-71, 87-87
🤖 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 `@docs/source/getting_started/windows/_installation_standalone.rst` at line 16, Update the CUDA dependency references in the Windows standalone installation document, including the entries around the CUDA Toolkit requirement and later cuDNN mentions, to use the exact capitalization “cuDNN” consistently.
🤖 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 22-24: Update the common skill references in the QAD skill
documentation to use the canonical repository-relative
`.agents/skills/common/...` paths, including `environment-setup.md`,
`workspace-management.md`, `slurm-setup.md`, and `remote-execution.md`; leave
the Megatron bridge paths unchanged.
In `@examples/llm_eval/lm_eval_hf.py`:
- Around line 298-308: Update the result-loading logic around the output_path
glob and scores extraction to support both directory paths and direct JSON file
paths. Load the selected JSON file directly when output_path is a file;
otherwise retain the existing latest-results-file selection. Extract the
requested task metrics from either the JSON's "results" or "groups" mapping,
preferring the available task entry before computing acc.
In `@examples/megatron_bridge/prune_minitron.py`:
- Around line 687-698: Move the accuracy-gate block in main immediately after
mtp.prune() completes and before either Megatron or Hugging Face output-saving
path writes artifacts. Preserve the existing score-bound evaluation and
sys.exit(1) behavior, ensuring failed pruning runs cannot leave outputs that
bypass the gate on rerun.
- Around line 242-247: Update the argparse definition for score_lower_bound in
prune_minitron.py to reject non-finite float values at the CLI boundary by
validating math.isfinite() after parsing. Preserve None as the unset value and
continue accepting finite thresholds.
In `@examples/torch_onnx/torch_quant_to_onnx.py`:
- Around line 125-136: Keep the recipe format and export mode consistent by
resolving a validated format profile once and using it throughout. In
examples/torch_onnx/torch_quant_to_onnx.py lines 125-136, derive Conv2d
overrides from the resolved recipe; document the compatibility contract in lines
458-468, reject incompatible recipe/mode combinations in lines 544-548, pass the
profile at lines 598-598, and use it for TensorRT guards at lines 613-625.
Update examples/torch_onnx/README.md lines 72-75 to document the enforced or
derived relationship, and add coverage for valid custom recipes and rejected
mismatches.
In `@modelopt/onnx/autocast/convert.py`:
- Around line 49-57: Update _capture_network_io_metadata and the corresponding
PrecisionConverter._restore_original_io_metadata flow so keep_io_types=True
remains valid after sanitization: preserve or reconcile renamed/disconnected
outputs and FP64 input/output element types before restoration, preventing
RuntimeError while retaining the original public I/O contract. Add regression
tests covering disconnected output names and FP64 I/O types.
In `@modelopt/onnx/autocast/precisionconverter.py`:
- Around line 307-420: Update _get_const_values to support Constant nodes
encoded with value_int, value_ints, or sparse_value, extracting their values for
shape inference without assuming attrs["value"] exists. When an encoding cannot
be safely converted to constant values, return None so _infer_unsqueeze_op_shape
exits safely instead of raising KeyError.
In `@tests/examples/llm_eval/test_llm_eval.py`:
- Around line 39-41: Add a focused test for _enforce_accuracy_gate that provides
a results JSON with a score below accuracy_lower_bound and asserts the function
raises SystemExit with code 1. Keep the existing passing-bound test unchanged so
both successful and failing gate behavior are covered.
In `@tests/unit/torch/export/test_quant_aware_conversion.py`:
- Around line 269-270: Add brief reason comments before each conditional local
Transformers import at tests/unit/torch/export/test_quant_aware_conversion.py
lines 180, 183, 237, 269-270, 297-298, and 358, explaining that transformers is
optional and conversion_mapping is version-dependent. Keep the existing
pytest.importorskip guards and imports unchanged.
In `@tools/launcher/core.py`:
- Around line 114-126: Preserve positional construction compatibility by moving
the new inline, reqs, and reqs_file fields in SandboxTask after all existing
fields, or making them keyword-only. In tools/launcher/slurm_config.py at lines
51-53 and 77-99, likewise append docker_user after the existing SlurmConfig
fields and slurm_factory parameters, or make it keyword-only.
In
`@tools/launcher/examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_prune.yaml`:
- Around line 23-25: Remove the hardcoded --trust_remote_code flag from all five
command invocations:
tools/launcher/examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_prune.yaml
lines 23-25 and 49-52, and mbridge_quantize.yaml lines 20-22, 31-34, and 49-52.
Keep these workflows disabled for remote code execution by default; if
compatibility requires the flag, replace it with an explicit opt-in parameter or
environment variable defaulting to false, with a pinned model revision and
security documentation.
In `@tools/launcher/tests/test_examples_resolve.py`:
- Around line 79-91: Update the task validation around the inline check in the
examples resolver to reject any non-empty args list when inline is set, matching
run_jobs() behavior. Use the existing task, inline, and args values and include
the task path/name in the assertion message; preserve validation for script
tasks and empty or absent args.
---
Nitpick comments:
In `@docs/source/getting_started/windows/_installation_standalone.rst`:
- Line 16: Update the CUDA dependency references in the Windows standalone
installation document, including the entries around the CUDA Toolkit requirement
and later cuDNN mentions, to use the exact capitalization “cuDNN” consistently.
In `@modelopt/onnx/autocast/precisionconverter.py`:
- Around line 1620-1665: Update _get_tensor_shape and _get_tensor_elem_type to
resolve initializers through self.initializer_map and value metadata through
self.value_info_map instead of scanning graph.initializer, input, output, and
value_info. Preserve the existing shape, element-type, Cast-producer fallback,
and missing-value behavior while relying on the maps rebuilt by
_refresh_gathernd_pre_cast_declarations.
In `@tests/examples/megatron_bridge/test_prune_minitron.py`:
- Around line 43-51: Extend the test case using num_nextn_predict_layers=1 and
mtp_hybrid_override_pattern to inspect both generated output formats: assert the
Megatron configuration or checkpoint metadata has MTP fields removed, and assert
the Hugging Face metadata persists the normalized hybrid pattern. Use each
format’s existing inspection/loading helpers rather than relying only on output
existence or parameter counts.
- Line 79: Add a focused test case in the existing pruning tests around the
score_lower_bound configuration, using a bound higher than the measured score
and the real pruning command implementation. Assert that this failing score-gate
path exits non-zero, while preserving the existing passing-path coverage.
In `@tests/examples/megatron_bridge/test_qad.py`:
- Around line 113-124: Update the test around the distillation command and
tracker assertions to validate duration-based exit independently: add a focused
configuration test or separate integration case with exit_interval unset, use a
controlled exit_duration_in_mins value, and retain assertions confirming the run
exits at the expected iteration/checkpoint. Ensure the existing interval-based
test remains scoped to interval exit rather than claiming duration coverage.
🪄 Autofix
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: 29d0031c-cd36-487c-9744-6092bdc07c37
📒 Files selected for processing (62)
.agents/skills/deployment/SKILL.md.agents/skills/deployment/references/benchmarking.md.agents/skills/deployment/references/support-matrix.md.agents/skills/evaluation/SKILL.md.agents/skills/evaluation/recipes/examples/example_eval.yaml.agents/skills/evaluation/recipes/examples/example_eval_next.yaml.agents/skills/evaluation/references/nel-next.md.agents/skills/qad/SKILL.md.claude/skills/qadCHANGELOG.rstdocs/source/getting_started/windows/_installation_standalone.rstexamples/hf_ptq/example_utils.pyexamples/llm_eval/lm_eval_hf.pyexamples/megatron_bridge/README.mdexamples/megatron_bridge/data/nemotron-cascade-2-blend.yamlexamples/megatron_bridge/distill.pyexamples/megatron_bridge/prune_minitron.pyexamples/torch_onnx/README.mdexamples/torch_onnx/hf_embedding_quant_to_onnx.pyexamples/torch_onnx/torch_quant_to_onnx.pyexamples/windows/onnx_ptq/genai_llm/README.mdexamples/windows/onnx_ptq/genai_llm/quantize.pymodelopt/onnx/autocast/convert.pymodelopt/onnx/autocast/precisionconverter.pymodelopt/onnx/utils.pymodelopt/torch/__init__.pymodelopt/torch/export/quant_aware_conversion.pymodelopt/torch/export/unified_export_hf.pymodelopt/torch/prune/plugins/mcore_minitron.pymodelopt/torch/puzzletron/anymodel/models/__init__.pymodelopt/torch/quantization/export_onnx.pymodelopt/torch/quantization/nn/modules/quant_linear.pymodelopt/torch/utils/dataset_utils.pymodelopt_recipes/huggingface/nemotron_llama/ptq/fp8_output_quant_proj.yamlmodelopt_recipes/huggingface/nemotron_llama/ptq/nvfp4_output_quant_proj.yamlmodelopt_recipes/ptq.mdnoxfile.pypyproject.tomltests/_test_utils/torch/transformers_models.pytests/examples/hf_ptq/test_example_utils.pytests/examples/llm_eval/test_llm_eval.pytests/examples/megatron_bridge/test_prune_minitron.pytests/examples/megatron_bridge/test_qad.pytests/examples/torch_onnx/test_hf_embedding_quant_to_onnx.pytests/examples/torch_onnx/test_torch_quant_to_onnx.pytests/gpu/torch/export/test_export_weight_gpu.pytests/gpu/torch/puzzletron/test_puzzletron.pytests/gpu_trtllm/torch/export/test_export_compressed_nvfp4.pytests/unit/onnx/autocast/test_precisionconverter.pytests/unit/torch/export/test_quant_aware_conversion.pytests/unit/torch/utils/test_dataset_utils.pytools/launcher/core.pytools/launcher/docs/configuration.mdtools/launcher/examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_prune.yamltools/launcher/examples/nvidia/NVIDIA-Nemotron-3-Nano-30B-A3B-BF16/mbridge_quantize.yamltools/launcher/slurm_config.pytools/launcher/tests/test_core.pytools/launcher/tests/test_core_extended.pytools/launcher/tests/test_docker_execution.pytools/launcher/tests/test_examples_resolve.pytools/launcher/tests/test_yaml_formats.pytools/mcp/pyproject.toml
💤 Files with no reviewable changes (1)
- tests/gpu/torch/puzzletron/test_puzzletron.py
| - `examples/megatron_bridge/{quantize.py,distill.py}` via `--help` | ||
| - `skills/common/{environment-setup,workspace-management,slurm-setup}.md`; also | ||
| `skills/common/remote-execution.md` for remote Slurm |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the canonical paths for common skills.
Lines 23-24 reference skills/common/.... The canonical skill tree is .agents/skills/common/.... The current paths can prevent the QAD skill from loading the required environment and Slurm instructions.
Proposed fix
- `skills/common/{environment-setup,workspace-management,slurm-setup}.md`; also
- `skills/common/remote-execution.md` for remote Slurm
+ `.agents/skills/common/{environment-setup,workspace-management,slurm-setup}.md`; also
+ `.agents/skills/common/remote-execution.md` for remote SlurmAs per path instructions, use relative paths from the repository root and keep .agents/ as the canonical source.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `examples/megatron_bridge/{quantize.py,distill.py}` via `--help` | |
| - `skills/common/{environment-setup,workspace-management,slurm-setup}.md`; also | |
| `skills/common/remote-execution.md` for remote Slurm | |
| - `.agents/skills/common/{environment-setup,workspace-management,slurm-setup}.md`; also | |
| `.agents/skills/common/remote-execution.md` for remote Slurm |
🤖 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 22 - 24, Update the common skill
references in the QAD skill documentation to use the canonical
repository-relative `.agents/skills/common/...` paths, including
`environment-setup.md`, `workspace-management.md`, `slurm-setup.md`, and
`remote-execution.md`; leave the Megatron bridge paths unchanged.
Source: Path instructions
| files = glob.glob(os.path.join(output_path, "**", "results*.json"), recursive=True) | ||
| if not files: | ||
| raise FileNotFoundError(f"No results*.json under {output_path}") | ||
| # Sort by mtime, not path: a reused output_path nests results under a | ||
| # <model_name>/ dir, and lexical order would pick the wrong run's file. | ||
| with open(max(files, key=os.path.getmtime)) as f: | ||
| scores = json.load(f)["results"].get(task, {}) | ||
| # lm-eval keys metrics by filter, e.g. "acc,none"; take acc (never acc_stderr). | ||
| acc = next((float(v) for k, v in scores.items() if k == "acc" or k.startswith("acc,")), None) | ||
| if acc is None: | ||
| raise KeyError(f"acc not found for '{task}' (have: {list(scores)})") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
get_file() {
gh api -X GET "repos/EleutherAI/lm-evaluation-harness/contents/$1?ref=v0.4.10" \
--jq '.content' | tr -d '\n' | base64 --decode
}
# Verify that v0.4.10 writes group results separately from task results.
get_file lm_eval/evaluator.py | rg -n -C 3 'groups|results|_to_eval_results'
# Verify the documented output-path behavior for the pinned release.
get_file docs/interface.md | rg -n -C 2 'output_path|JSON file'Repository: NVIDIA/Model-Optimizer
Length of output: 5852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- CONTRIBUTING coding standards ---'
rg -n -A80 -B5 'coding standards|pre-commit hooks' CONTRIBUTING.md | head -n 180
echo '--- target implementation ---'
cat -n examples/llm_eval/lm_eval_hf.py | sed -n '220,325p'
echo '--- related tests and call sites ---'
rg -n -C4 'accuracy_threshold|results\*\.json|output_path|mmlu' examples tests 2>/dev/null | head -n 240Repository: NVIDIA/Model-Optimizer
Length of output: 30609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
path = Path("examples/llm_eval/lm_eval_hf.py")
text = path.read_text()
for needle in ("glob.glob", '["results"]', '["groups"]', "accuracy_threshold", 'tasks="mmlu"', "--output_path"):
print(f"{needle!r}: {text.count(needle)}")
# Model the exact current lookup against representative lm-eval reports.
reports = [
{"results": {"mmlu": {"acc,none": 0.5}}},
{"groups": {"mmlu": {"acc,none": 0.5}}},
{"results": {"mmlu": {"acc": 0.5}}, "groups": {"mmlu": {"acc": 0.4}}},
]
for report in reports:
scores = report.get("results", {}).get("mmlu", {})
try:
acc = next(
float(v) for k, v in scores.items()
if k == "acc" or k.startswith("acc,")
)
except StopIteration:
acc = None
print(json.dumps(report), "-> current lookup:", acc)
PYRepository: NVIDIA/Model-Optimizer
Length of output: 490
Support JSON output files and group metrics.
When --output_path names a JSON file, line 298 treats it as a directory. For group tasks such as mmlu, the aggregate acc can be stored under groups, but line 304 reads only results. Read the task from results or groups, and support direct JSON file paths.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 302-302: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(max(files, key=os.path.getmtime))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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/llm_eval/lm_eval_hf.py` around lines 298 - 308, Update the
result-loading logic around the output_path glob and scores extraction to
support both directory paths and direct JSON file paths. Load the selected JSON
file directly when output_path is a file; otherwise retain the existing
latest-results-file selection. Extract the requested task metrics from either
the JSON's "results" or "groups" mapping, preferring the available task entry
before computing acc.
| parser.add_argument( | ||
| "--score_lower_bound", | ||
| type=float, | ||
| default=None, | ||
| help="If set, fail the job when the NAS-based pruned model's score is below this bound.", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-finite score bounds.
type=float accepts nan, inf, and -inf. nan makes every finite score fail. -inf makes every finite score pass. Validate math.isfinite() at the argument boundary.
Suggested validation
+def _finite_float(value: str) -> float:
+ parsed = float(value)
+ if not math.isfinite(parsed):
+ raise argparse.ArgumentTypeError("must be finite")
+ return parsed
+
...
- type=float,
+ type=_finite_float,Based on the new CLI boundary, reject values that cannot represent a valid score threshold.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| parser.add_argument( | |
| "--score_lower_bound", | |
| type=float, | |
| default=None, | |
| help="If set, fail the job when the NAS-based pruned model's score is below this bound.", | |
| ) | |
| def _finite_float(value: str) -> float: | |
| parsed = float(value) | |
| if not math.isfinite(parsed): | |
| raise argparse.ArgumentTypeError("must be finite") | |
| return parsed | |
| parser.add_argument( | |
| "--score_lower_bound", | |
| type=_finite_float, | |
| default=None, | |
| help="If set, fail the job when the NAS-based pruned model's score is below this bound.", | |
| ) |
🤖 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/prune_minitron.py` around lines 242 - 247, Update
the argparse definition for score_lower_bound in prune_minitron.py to reject
non-finite float values at the CLI boundary by validating math.isfinite() after
parsing. Preserve None as the unset value and continue accepting finite
thresholds.
| # Accuracy gate: exit non-zero if pruned model's score is below the bound | ||
| if args.score_lower_bound is not None: | ||
| best_score = pruning_scores["best"].get("score") | ||
| assert best_score is not None, "No scored best candidate in pruning_scores" | ||
| passed = best_score >= args.score_lower_bound | ||
| print_rank_0( | ||
| f"[score_gate] final pruned model {args.prune_score_func} score = {best_score:.4f} " | ||
| f"(lower_bound {args.score_lower_bound}) -> {'PASS' if passed else 'FAIL'}" | ||
| ) | ||
| if not passed: | ||
| sys.exit(1) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Run the accuracy gate before saving output artifacts.
The gate runs after the Megatron and Hugging Face output paths have written artifacts. If the score is below the bound, sys.exit(1) leaves a model and configuration behind. On a rerun, the earlier output-existence check returns successfully before evaluating the score. A below-bound artifact can therefore bypass the gate.
Move the gate immediately after mtp.prune() and before saving, or remove failed artifacts and make existing outputs re-enter the gate.
Based on the existing output-existence guard in main, the current ordering can turn a failed gate into a successful rerun.
🤖 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/prune_minitron.py` around lines 687 - 698, Move the
accuracy-gate block in main immediately after mtp.prune() completes and before
either Megatron or Hugging Face output-saving path writes artifacts. Preserve
the existing score-bound evaluation and sys.exit(1) behavior, ensuring failed
pruning runs cannot leave outputs that bypass the gate on rerun.
| def get_quant_config(quantize_mode, recipe=None): | ||
| """Get quantization config, overriding Conv2d for TRT compatibility. | ||
|
|
||
| TensorRT only supports FP8 and INT8 for Conv layers. | ||
| The config is loaded from ``recipe`` when given, else from the preset YAML | ||
| matching ``quantize_mode``. TensorRT only supports FP8 and INT8 for Conv layers. | ||
| - For FP8: add MHA-aware LayerNorm output quantizer so TRT fuses shared Q/DQ into | ||
| downstream attention matmuls. Softmax-output Q/DQ is inserted by the FP8 ONNX | ||
| exporter's post-processing (fixed 1/448 scale, no calibration needed). | ||
| - For MXFP8, NVFP4: override Conv2d to FP8 | ||
| - For INT4_AWQ: override Conv2d to INT8 | ||
| """ | ||
| config: dict = copy.deepcopy(QUANT_CONFIG_DICT[quantize_mode]) | ||
| config: dict = load_quant_config(recipe or quantize_mode) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Keep the recipe format and export mode consistent.
--recipe can replace the loaded quantization format, but Conv2d overrides and TensorRT guards still use --quantize_mode. For example, --quantize_mode=fp8 --recipe=nvfp4 loads NVFP4 settings but skips the NVFP4 high-rank dynamic-quantizer guard. Derive format-specific behavior from validated recipe metadata, or reject incompatible pairs before quantization. Add coverage for both allowed custom recipes and rejected mismatches.
examples/torch_onnx/torch_quant_to_onnx.py#L125-L136: select Conv2d overrides from the resolved recipe format.examples/torch_onnx/torch_quant_to_onnx.py#L458-L468: define the compatibility contract in the CLI help.examples/torch_onnx/torch_quant_to_onnx.py#L544-L548: reject incompatible recipe and mode combinations.examples/torch_onnx/torch_quant_to_onnx.py#L598-L598: pass a resolved format profile with the recipe configuration.examples/torch_onnx/torch_quant_to_onnx.py#L613-L625: use the resolved format profile for TensorRT export guards.examples/torch_onnx/README.md#L72-L75: document the enforced or derived recipe/mode relationship.
📍 Affects 2 files
examples/torch_onnx/torch_quant_to_onnx.py#L125-L136(this comment)examples/torch_onnx/torch_quant_to_onnx.py#L458-L468examples/torch_onnx/torch_quant_to_onnx.py#L544-L548examples/torch_onnx/torch_quant_to_onnx.py#L598-L598examples/torch_onnx/torch_quant_to_onnx.py#L613-L625examples/torch_onnx/README.md#L72-L75
🤖 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/torch_onnx/torch_quant_to_onnx.py` around lines 125 - 136, Keep the
recipe format and export mode consistent by resolving a validated format profile
once and using it throughout. In examples/torch_onnx/torch_quant_to_onnx.py
lines 125-136, derive Conv2d overrides from the resolved recipe; document the
compatibility contract in lines 458-468, reject incompatible recipe/mode
combinations in lines 544-548, pass the profile at lines 598-598, and use it for
TensorRT guards at lines 613-625. Update examples/torch_onnx/README.md lines
72-75 to document the enforced or derived relationship, and add coverage for
valid custom recipes and rejected mismatches.
| # Exercise the accuracy gate: reads the results file and enforces the bound | ||
| output_path=str(tmp_path / "results"), | ||
| accuracy_lower_bound=0.1, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover the failing accuracy-gate path.
This test only uses a passing bound. A regression that removes the non-zero exit for a failing score will still pass. Add a focused test with a result JSON below the bound and assert SystemExit(1) from _enforce_accuracy_gate.
As per path instructions, “Tests must exercise the behavior they claim to validate.”
🤖 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 `@tests/examples/llm_eval/test_llm_eval.py` around lines 39 - 41, Add a focused
test for _enforce_accuracy_gate that provides a results JSON with a score below
accuracy_lower_bound and asserts the function raises SystemExit with code 1.
Keep the existing passing-bound test unchanged so both successful and failing
gate behavior are covered.
Source: Path instructions
| pytest.importorskip("transformers.core_model_loading") | ||
| from transformers.core_model_loading import WeightRenaming |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '1,340p' tests/unit/torch/export/test_quant_aware_conversion.py
printf '%s\n' '--- relevant contribution guidance ---'
rg -n -A8 -B4 'coding standards|imports|optional depend|unit tests|writing and running tests' CONTRIBUTING.mdRepository: NVIDIA/Model-Optimizer
Length of output: 20597
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/unit/torch/export/test_quant_aware_conversion.py")
source = path.read_text()
tree = ast.parse(source)
lines = source.splitlines()
print("Optional-import sites and preceding source lines:")
for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom) and node.module == "transformers.core_model_loading":
start = max(0, node.lineno - 4)
print(f"\nimport line {node.lineno}: {lines[node.lineno - 1].strip()}")
for number in range(start + 1, node.lineno):
print(f"{number}: {lines[number - 1].strip()}")
print("\nAll pytest.importorskip calls:")
for node in ast.walk(tree):
if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute):
if isinstance(node.func.value, ast.Name) and node.func.value.id == "pytest" and node.func.attr == "importorskip":
argument = ast.get_source_segment(source, node.args[0]) if node.args else ""
print(f"line {node.lineno}: pytest.importorskip({argument})")
PYRepository: NVIDIA/Model-Optimizer
Length of output: 1364
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- all optional-import contexts ---'
sed -n '228,305p' tests/unit/torch/export/test_quant_aware_conversion.py
sed -n '348,365p' tests/unit/torch/export/test_quant_aware_conversion.pyRepository: NVIDIA/Model-Optimizer
Length of output: 4591
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import ast
path = Path("tests/unit/torch/export/test_quant_aware_conversion.py")
source = path.read_text()
lines = source.splitlines()
tree = ast.parse(source)
parents = {}
for parent in ast.walk(tree):
for child in ast.iter_child_nodes(parent):
parents[id(child)] = parent
print("All imports nested inside test functions:")
for node in ast.walk(tree):
if isinstance(node, (ast.Import, ast.ImportFrom)):
parent = parents.get(id(node))
while parent is not None and not isinstance(parent, (ast.FunctionDef, ast.AsyncFunctionDef)):
parent = parents.get(id(parent))
if parent is not None:
text = ast.get_source_segment(source, node).replace("\n", " ")
preceding = lines[node.lineno - 2].strip() if node.lineno >= 2 else ""
print(f"line {node.lineno}, function {parent.name}: {text}")
print(f" preceding line: {preceding!r}")
PYRepository: NVIDIA/Model-Optimizer
Length of output: 1421
Document conditional Transformers imports.
Add a brief reason comment before each local Transformers import at lines 180, 183, 237, 270, 298, and 358. Keep the imports conditional because transformers is optional and conversion_mapping is version-dependent.
📍 Affects 1 file
tests/unit/torch/export/test_quant_aware_conversion.py#L269-L270(this comment)tests/unit/torch/export/test_quant_aware_conversion.py#L297-L298
🤖 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 `@tests/unit/torch/export/test_quant_aware_conversion.py` around lines 269 -
270, Add brief reason comments before each conditional local Transformers import
at tests/unit/torch/export/test_quant_aware_conversion.py lines 180, 183, 237,
269-270, 297-298, and 358, explaining that transformers is optional and
conversion_mapping is version-dependent. Keep the existing pytest.importorskip
guards and imports unchanged.
Sources: Coding guidelines, Path instructions
| script: str = None | ||
| # Inline shell command run instead of `script` (mutually exclusive; setting | ||
| # `args` too is rejected — put everything in the command). Lets one-liner jobs | ||
| # live in the YAML without a wrapper .sh. Must be a SINGLE line: the --yaml CLI | ||
| # layer rejects multi-line values, so YAMLs use a folded scalar (>-) and `&&`. | ||
| inline: str = None | ||
| # pip requirements installed in the container before the command runs | ||
| # (`pip install [-r reqs_file] [reqs] && <command>`). `reqs` is a raw | ||
| # pip-install arg string (e.g. "transformers<5 fire"); `reqs_file` is a | ||
| # requirements.txt path relative to the run dir (e.g. | ||
| # modules/Model-Optimizer/examples/llm_eval/requirements.txt). | ||
| reqs: str = None | ||
| reqs_file: str = None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline tools/launcher/core.py --items all
ast-grep outline tools/launcher/slurm_config.py --items all
rg -n -C 3 --glob '*.py' \
'\b(SandboxTask[0-4]?|SlurmConfig|slurm_factory)\s*\(' .Repository: NVIDIA/Model-Optimizer
Length of output: 28282
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- CONTRIBUTING coding standards ---'
rg -n -A80 -B5 -- 'coding standards|pre-commit hooks|submitting your code' CONTRIBUTING.md
printf '%s\n' '--- SandboxTask definition ---'
cat -n tools/launcher/core.py | sed -n '105,135p'
printf '%s\n' '--- SlurmConfig and slurm_factory definitions ---'
cat -n tools/launcher/slurm_config.py | sed -n '25,115p'
printf '%s\n' '--- focused diff ---'
git diff -- tools/launcher/core.py tools/launcher/slurm_config.pyRepository: NVIDIA/Model-Optimizer
Length of output: 12348
Preserve positional configuration construction.
The new fields change positional argument mapping:
- Append
inline,reqs, andreqs_fileafter the existingSandboxTaskfields, or make them keyword-only. - Append
docker_userafter the existingSlurmConfigfields andslurm_factory()parameters, or make it keyword-only.
📍 Affects 2 files
tools/launcher/core.py#L114-L126(this comment)tools/launcher/slurm_config.py#L51-L53tools/launcher/slurm_config.py#L77-L99
🤖 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 `@tools/launcher/core.py` around lines 114 - 126, Preserve positional
construction compatibility by moving the new inline, reqs, and reqs_file fields
in SandboxTask after all existing fields, or making them keyword-only. In
tools/launcher/slurm_config.py at lines 51-53 and 77-99, likewise append
docker_user after the existing SlurmConfig fields and slurm_factory parameters,
or make it keyword-only.
| inline = task.get("inline") | ||
| # A task runs either a `script:` wrapper or an `inline:` command — exactly one. | ||
| assert (isinstance(script, str) and script.strip()) or ( | ||
| isinstance(inline, str) and inline.strip() | ||
| ), f"{path}:{name}: task needs a `script` or `inline`" | ||
| assert not (script and inline), f"{path}:{name}: set only one of `script`/`inline`" | ||
|
|
||
| # nemo-run's --yaml CLI layer rejects multi-line override values, so an | ||
| # inline command must stay single-line (use a folded scalar `>-`). | ||
| if inline: | ||
| assert "\n" not in inline.strip(), ( | ||
| f"{path}:{name}: `inline` must be single-line (use folded `>-`, chain with `&&`)" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-empty args for inline tasks.
run_jobs() rejects non-empty args when inline is set. This validator only checks that args is a list. Validate this combination here so invalid example YAML fails before job submission.
Proposed fix
inline = task.get("inline")
+ args = task.get("args")
# A task runs either a `script:` wrapper or an `inline:` command — exactly one.
assert (isinstance(script, str) and script.strip()) or (
isinstance(inline, str) and inline.strip()
), f"{path}:{name}: task needs a `script` or `inline`"
assert not (script and inline), f"{path}:{name}: set only one of `script`/`inline`"
+ assert not (inline and args), f"{path}:{name}: `args` is only valid with `script`"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| inline = task.get("inline") | |
| # A task runs either a `script:` wrapper or an `inline:` command — exactly one. | |
| assert (isinstance(script, str) and script.strip()) or ( | |
| isinstance(inline, str) and inline.strip() | |
| ), f"{path}:{name}: task needs a `script` or `inline`" | |
| assert not (script and inline), f"{path}:{name}: set only one of `script`/`inline`" | |
| # nemo-run's --yaml CLI layer rejects multi-line override values, so an | |
| # inline command must stay single-line (use a folded scalar `>-`). | |
| if inline: | |
| assert "\n" not in inline.strip(), ( | |
| f"{path}:{name}: `inline` must be single-line (use folded `>-`, chain with `&&`)" | |
| ) | |
| inline = task.get("inline") | |
| args = task.get("args") | |
| # A task runs either a `script:` wrapper or an `inline:` command — exactly one. | |
| assert (isinstance(script, str) and script.strip()) or ( | |
| isinstance(inline, str) and inline.strip() | |
| ), f"{path}:{name}: task needs a `script` or `inline`" | |
| assert not (script and inline), f"{path}:{name}: set only one of `script`/`inline`" | |
| assert not (inline and args), f"{path}:{name}: `args` is only valid with `script`" | |
| # nemo-run's --yaml CLI layer rejects multi-line override values, so an | |
| # inline command must stay single-line (use a folded scalar `>-`). | |
| if inline: | |
| assert "\n" not in inline.strip(), ( | |
| f"{path}:{name}: `inline` must be single-line (use folded `>-`, chain with `&&`)" | |
| ) |
🤖 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 `@tools/launcher/tests/test_examples_resolve.py` around lines 79 - 91, Update
the task validation around the inline check in the examples resolver to reject
any non-empty args list when inline is set, matching run_jobs() behavior. Use
the existing task, inline, and args values and include the task path/name in the
assertion message; preserve validation for script tasks and empty or absent
args.
Cherry-picked PRs
Summary by CodeRabbit
New Features
Bug Fixes
Documentation