fix(hf_ptq): use no_grad instead of inference_mode in export_quantized (NVBug 6537702) - #2047
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesFSDP2 export
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2047 +/- ##
==========================================
+ Coverage 65.45% 66.96% +1.50%
==========================================
Files 521 521
Lines 59857 59857
==========================================
+ Hits 39181 40081 +900
+ Misses 20676 19776 -900
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:
|
cjluo-nv
left a comment
There was a problem hiding this comment.
Bot review (bedrock-claude-opus-5) — DM the bot to share feedback.
One-line fix (torch.inference_mode() → torch.no_grad() in examples/hf_ptq/hf_ptq.py::export_quantized) plus a CHANGELOG entry and a new CPU-only test. The root-cause analysis matches what I see in the tree: the FSDP2 export gathers full params inside the context via get_model_state_dict(full_state_dict=True) (intercepted in modelopt/torch/opt/_hooks.py::_get_model_state_dict_with_dm_check), and inference tensors can't have their version counter set on the subsequent param.detach(). no_grad still disables autograd and the export path does no training-style work, so behavior for the non-FSDP2 paths should be unchanged. Diff is tiny, backward compatible, and the new test file's header matches the canonical LICENSE_HEADER (no licensing concern). No prompt-injection content in the PR metadata.
Flagging for owner sign-off on the points below rather than approving, since the actual failure mode can't be exercised in CI and the added test only checks source text.
| # Use no_grad rather than inference_mode: on the FSDP2 path the export gathers the | ||
| # full params inside this context, and tensors created under inference_mode are | ||
| # inference tensors whose version counter cannot be set, so the subsequent | ||
| # state_dict() -> param.detach() fails with | ||
| # "RuntimeError: Cannot set version_counter for inference tensor". |
There was a problem hiding this comment.
nit: could we trim down the comment?
| # Use no_grad rather than inference_mode: on the FSDP2 path the export gathers the | |
| # full params inside this context, and tensors created under inference_mode are | |
| # inference tensors whose version counter cannot be set, so the subsequent | |
| # state_dict() -> param.detach() fails with | |
| # "RuntimeError: Cannot set version_counter for inference tensor". | |
| # FSDP2 export path gathers full params in this context and inference tensors break state_dict()'s | |
| # detach here. |
sugunav14
left a comment
There was a problem hiding this comment.
LGTM, left a minor comment
5b26505 to
c32496e
Compare
`export_quantized` wrapped its whole body in `torch.inference_mode()`. On the FSDP2 path (`--use_fsdp2`), `get_model_state_dict(full_state_dict=True)` gathers the full params inside that context, so the gathered tensors are inference tensors; the subsequent `state_dict()` -> `param.detach()` then fails with `RuntimeError: Cannot set version_counter for inference tensor`. Switching the export context to `torch.no_grad()` keeps the gathered params as normal tensors (version counter intact) so `detach()` works, while still disabling autograd. Original fix by Shengliang Xu, verified end-to-end on 2 nodes with dense Qwen3-8B and Qwen3-30B-A3B (MoE) FSDP2 PTQ fp8 exports. Fixes NVBug 6537702 (Llama-3.1-8B-Instruct, 2x8 GB200, fp8_default-kv_fp8). Co-authored-by: Shengliang Xu <shengliangx@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Zhiyu Cheng <zhiyuc@nvidia.com>
c32496e to
fb737b5
Compare
|
…d (NVBug 6537702) (#2047) ### What does this PR do? Type of change: Bug fix Fixes [NVBug 6537702](https://nvbugspro.nvidia.com/bug/6537702) / [OMNIML-5658](https://jirasw.nvidia.com/browse/OMNIML-5658) — multi-node FSDP2 PTQ export fails on all ranks: ``` hf_ptq.py:910 export_quantized -> export_hf_checkpoint unified_export_hf.py:1446 _export_transformers_checkpoint -> get_model_state_dict modelopt/torch/opt/_hooks.py:88 _get_model_state_dict_with_dm_check torch/distributed/checkpoint/state_dict.py:481 _get_model_state_dict torch/nn/modules/module.py:2160 _save_to_state_dict destination[prefix + name] = param if keep_vars else param.detach() RuntimeError: Cannot set version_counter for inference tensor ``` **Root cause.** `export_quantized` wrapped its whole body in `torch.inference_mode()`. On the FSDP2 path (`--use_fsdp2`), `get_model_state_dict(full_state_dict=True)` gathers the full params *inside* that context, so the gathered tensors are inference tensors. Inference tensors have no version counter, so the subsequent `state_dict()` → `param.detach()` raises. **Fix.** Use `torch.no_grad()` for the export context. It still disables autograd, but the gathered params stay normal tensors with an intact version counter, so `detach()` works. FSDP2-only failure — the non-FSDP2 path never hit it because its params already exist outside the context. The one-line fix is originally by @shengliangx (`b0e4328` on `shengliangx/distributed-unified`); this PR retargets it to the post-rename `examples/hf_ptq/` path and adds a changelog entry and a regression guard. ### Usage ```bash # 2 nodes x 8 GB200, previously failed at export on every rank torchrun --nnodes=2 --node_rank=0 --master_addr=$MASTER --master_port=6000 --nproc_per_node=8 \ hf_ptq.py --model Llama-3.1-8B-Instruct --dataset cnn_dailymail \ --recipe general/ptq/fp8_default-kv_fp8 --batch_size 8 --calib_size 512 \ --export_path ./Llama-3.1-8B-Instruct-fp8_default-kv_fp8 --use_fsdp2 ``` ### Testing - End-to-end on 2 nodes by @shengliangx on the original branch: dense Qwen3-8B and Qwen3-30B-A3B (MoE) FSDP2 PTQ fp8 checkpoints export successfully. - Added `tests/examples/hf_ptq/test_export_quantized_context.py`, a CPU-only guard asserting `export_quantized` enters `torch.no_grad()` and not `torch.inference_mode()`. A functional regression test would need a 2-node FSDP2 job, which CI does not run, so this encodes the invariant instead. - `pre-commit run --files` clean on all three changed files. Reporter (Kenny Kang, GPU SWQA) still needs to confirm on the original 2x8 GB200 Llama-3.1-8B repro. ### 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](https://github.com/NVIDIA/Model-Optimizer/blob/main/CHANGELOG.rst)?: ✅ - Did you get Claude approval on this PR?: ❌ ### Additional Information Keyword `Committed_ModelOpt_0.46.0` on the bug — should land for 0.46. 🤖 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** * Fixed multi-node quantized model exports to prevent runtime errors when gathering and detaching parameters. * Improved compatibility with FSDP2 during Hugging Face PTQ exports. * **Tests** * Added coverage to verify the export process uses the compatible gradient context. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Zhiyu Cheng <zhiyuc@nvidia.com> Co-authored-by: Shengliang Xu <shengliangx@nvidia.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
What does this PR do?
Type of change: Bug fix
Fixes NVBug 6537702 / OMNIML-5658 — multi-node FSDP2 PTQ export fails on all ranks:
Root cause.
export_quantizedwrapped its whole body intorch.inference_mode(). On the FSDP2 path (--use_fsdp2),get_model_state_dict(full_state_dict=True)gathers the full params inside that context, so the gathered tensors are inference tensors. Inference tensors have no version counter, so the subsequentstate_dict()→param.detach()raises.Fix. Use
torch.no_grad()for the export context. It still disables autograd, but the gathered params stay normal tensors with an intact version counter, sodetach()works. FSDP2-only failure — the non-FSDP2 path never hit it because its params already exist outside the context.The one-line fix is originally by @shengliangx (
b0e4328onshengliangx/distributed-unified); this PR retargets it to the post-renameexamples/hf_ptq/path and adds a changelog entry and a regression guard.Usage
Testing
tests/examples/hf_ptq/test_export_quantized_context.py, a CPU-only guard assertingexport_quantizedenterstorch.no_grad()and nottorch.inference_mode(). A functional regression test would need a 2-node FSDP2 job, which CI does not run, so this encodes the invariant instead.pre-commit run --filesclean on all three changed files.Reporter (Kenny Kang, GPU SWQA) still needs to confirm on the original 2x8 GB200 Llama-3.1-8B repro.
Before your PR is "Ready for review"
CONTRIBUTING.md: N/AAdditional Information
Keyword
Committed_ModelOpt_0.46.0on the bug — should land for 0.46.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests