Skip to content

[6309094] updated readme to clarify simple_qat_train.py usecase - #1742

Merged
kinjalpatel27 merged 2 commits into
mainfrom
kinjal/qat_multigpu
Jun 15, 2026
Merged

[6309094] updated readme to clarify simple_qat_train.py usecase#1742
kinjalpatel27 merged 2 commits into
mainfrom
kinjal/qat_multigpu

Conversation

@kinjalpatel27

@kinjalpatel27 kinjalpatel27 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: documentation

Added clarification about simjple_qat_train.py which is a demonstration of QAT flow and not meant for multi-GPU training

Before your PR is "Ready for review"

Make sure you read and follow Contributor guidelines and your commits are signed (git commit -s -S).

Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded trust_remote_code=True, torch.load(..., weights_only=False), pickle, etc.).

  • Is this change backward compatible?: N/A
  • 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
  • Did you update Changelog?: N/A
  • Did you get Claude approval on this PR?: ✅ / ❌ / N/A

Additional Information

Summary by CodeRabbit

  • Documentation
    • Expanded the QAT/QAD README note section with clearer, end-to-end minimal demo instructions, highlighting a single-GPU quantize+train+save workflow via a dedicated script.
    • Added guidance for multi-GPU training using accelerate launch, with a pointer to the appropriate training entry point.

Signed-off-by: Kinjal Patel <kinjalpravin@nvidia.com>
@kinjalpatel27
kinjalpatel27 requested a review from a team as a code owner June 15, 2026 19:54
@kinjalpatel27
kinjalpatel27 requested a review from mxinO June 15, 2026 19:54
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The NOTE section in examples/llm_qat/README.md (lines 91–97) is expanded to introduce simple_qat_train.py as a minimal single-GPU quantize-train-save script, provide an example command, and add a follow-up pointer to multi-GPU training using train.py with accelerate launch.

Changes

QAT README NOTE section update

Layer / File(s) Summary
Expanded NOTE block with single-GPU and multi-GPU instructions
examples/llm_qat/README.md
NOTE block now describes simple_qat_train.py as a minimal single-GPU end-to-end QAT/QAD demo with an example command, and adds a sentence pointing to multi-GPU training via train.py + accelerate launch.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Security Anti-Patterns ❌ Error New non-permissive PIP dependency py7zr (LGPL v2.1) added in examples/llm_qat/requirements.txt without required approval or justification from @NVIDIA/modelopt-setup-codeowners. Remove py7zr or provide explicit approval and justification from @NVIDIA/modelopt-setup-codeowners in PR description citing valid licensing concerns and project necessity.
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating the README to clarify the usecase of simple_qat_train.py, which aligns with the file modifications and PR objectives.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kinjal/qat_multigpu

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

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

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

👉 Steps to fix this

Actionable comments posted: 1

🤖 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 `@examples/llm_qat/README.md`:
- Line 97: The link fragment in the line containing the text about multi-GPU
training with accelerate launch references an incorrect anchor. Update the
broken link fragment from `#Run-End-to-End-QAT/QAD-Example` to
`#run-end-to-end-qatqad-example` to match the normalized Markdown anchor generated
from the actual heading. Markdown automatically converts heading text to
lowercase anchors and replaces spaces with hyphens while removing special
characters like forward slashes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 08e479e9-5793-4c24-ac13-ee0c37ea5c1b

📥 Commits

Reviewing files that changed from the base of the PR and between 0f61c98 and c6b00d3.

📒 Files selected for processing (1)
  • examples/llm_qat/README.md

Comment thread examples/llm_qat/README.md Outdated
Signed-off-by: Kinjal Patel <kinjalpravin@nvidia.com>
@kinjalpatel27

Copy link
Copy Markdown
Contributor Author

/claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude review passed — no blocking issues found. LGTM

Documentation-only change (3 additions / 1 deletion in examples/llm_qat/README.md). Verified that the referenced files (simple_qat_train.py, train.py) exist and the #qat anchor resolves to the ### QAT heading. No algorithm, mode/state, export, or backward-compatibility surface area is touched.

@cjluo-nv cjluo-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bot review — DM the bot to share feedback.

Documentation-only change (+3/-1) clarifying that simple_qat_train.py is a single-GPU, end-to-end QAT demo (no transformer trainer) and pointing to train.py + accelerate launch for multi-GPU. Verified against the source: simple_qat_train.py exists, uses a plain training loop with a single .cuda() call (single-GPU, no Trainer), and train.py exists with accelerate launch usage. The #qat anchor maps correctly to the ### QAT heading. Links are valid, no logic/licensing changes, and no injection attempts in the untrusted content.

@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.07%. Comparing base (6968fe7) to head (0b7e7a1).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1742      +/-   ##
==========================================
- Coverage   77.11%   77.07%   -0.04%     
==========================================
  Files         511      511              
  Lines       56259    56289      +30     
==========================================
+ Hits        43383    43386       +3     
- Misses      12876    12903      +27     
Flag Coverage Δ
examples 41.84% <ø> (-0.11%) ⬇️
unit 54.39% <ø> (+<0.01%) ⬆️

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

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

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

@cjluo-nv cjluo-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Bot review — DM the bot to share feedback.

Documentation-only change (+3/-1) to examples/llm_qat/README.md clarifying that simple_qat_train.py is a single-GPU, end-to-end QAT demo (no transformer trainer) and pointing to train.py + accelerate launch for multi-GPU. Verified: both simple_qat_train.py and train.py exist, and the #qat anchor resolves to the ### QAT heading. The previously-flagged broken anchor (#Run-End-to-End-QAT/QAD-Example, CodeRabbit MD051, minor) has been addressed — the author switched the link to the valid #qat fragment. No logic, no licensing changes, and no prompt-injection in the untrusted content.

@kinjalpatel27
kinjalpatel27 merged commit fa94e6e into main Jun 15, 2026
49 checks passed
@kinjalpatel27
kinjalpatel27 deleted the kinjal/qat_multigpu branch June 15, 2026 22:14
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-06-15 22:15 UTC

@kevalmorabia97 kevalmorabia97 added the cherry-pick-done Added by bot once PR is cherry-picked to the release branch label Jun 22, 2026
kevalmorabia97 added a commit that referenced this pull request Jun 23, 2026
#1755 #1754 #1761 #1765 (#1793)

## Cherry-picked PRs

- #1660
- #1742
- #1740
- #1744
- #1737
- #1669
- #1690
- #1746
- #1750
- #1755
- #1754
- #1761
- #1765

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
  * Added Gemma 4 multimodal PTQ recipe with W4A8 quantization
* Added support for hybrid Nemotron model export/import (Mamba +
Attention + MoE)

* **Bug Fixes**
  * Fixed checkpoint export for Llama4 MoE quantized models
  * Improved Mamba convolution parameter compatibility
  * Fixed Windows C++ extension compilation
  * Enhanced chat template error messages

* **Documentation**
  * Expanded version 0.45 release notes with detailed feature entries
  * Updated quantization and pruning tutorials with refined parameters
  * Improved Nemotron-3-Nano evaluation and deployment guidance

* **Deprecated/Breaking Changes**
* KDTrainer: eval_loss now reports KD loss; CE loss moved to secondary
metric
  * Increased Transformers minimum version to >=5.3

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
Signed-off-by: Kinjal Patel <kinjalpravin@nvidia.com>
Signed-off-by: Shengliang Xu <shengliangx@nvidia.com>
Signed-off-by: dimapihtar <dpykhtar@nvidia.com>
Signed-off-by: Zhiyu Cheng <zhiyuc@nvidia.com>
Signed-off-by: Jennifer Chen <jennifchen@nvidia.com>
Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
Signed-off-by: realAsma <akuriparambi@nvidia.com>
Signed-off-by: vipandya <vipandya@nvidia.com>
Signed-off-by: Sabari07 <sabursd18@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: kinjalpatel27 <31936134+kinjalpatel27@users.noreply.github.com>
Co-authored-by: Shengliang Xu <106840466+shengliangxu@users.noreply.github.com>
Co-authored-by: Dmytro Pykhtar <37850217+dimapihtar@users.noreply.github.com>
Co-authored-by: Zhiyu <zhiyuc@nvidia.com>
Co-authored-by: Jenny Chen <jennifchen@nvidia.com>
Co-authored-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
Co-authored-by: realAsma <86726418+realAsma@users.noreply.github.com>
Co-authored-by: vishalpandya1990 <vishalpandya1990@gmail.com>
Co-authored-by: Sabari07 <sabursd18@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-pick-done Added by bot once PR is cherry-picked to the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants