Skip to content

[Docs] Minor fixes for sequential onloading docs - #2771

Open
kylesayrs wants to merge 2 commits into
mainfrom
kylesayrs/seq-docs-fix
Open

[Docs] Minor fixes for sequential onloading docs#2771
kylesayrs wants to merge 2 commits into
mainfrom
kylesayrs/seq-docs-fix

Conversation

@kylesayrs

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
@kylesayrs
kylesayrs marked this pull request as ready for review May 29, 2026 15:33
@kylesayrs kylesayrs added the ready When a PR is ready for full CI testing before merge label May 29, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review.

Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed.

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: eeb15f6f-2901-4823-abd7-08b10c2487dd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR updates the sequential onloading documentation guide with two changes: correcting the pseudo-code example to show layer operations applied to activations[i] rather than dataset[i], and adding a warning that highly granular sequential_targets values like "Linear" are not supported for NVFP4/NVFP4A16 quantization due to fused-weight constraints.

Changes

Sequential Onloading Guide

Layer / File(s) Summary
Sequential onloading guide updates
docs/guides/big_models_and_distributed/sequential_onloading.md
Pseudo-code example corrected to apply layer operations to activations[i]. Warning added clarifying that granular sequential targets are unsupported for NVFP4/NVFP4A16 quantization.

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relevance to the changeset. Consider adding a description explaining the specific fixes made, such as the pseudo-code correction and the NVFP4 quantization limitation warning.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the changeset as minor documentation fixes to sequential onloading, matching the actual updates to the documentation file.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch kylesayrs/seq-docs-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request corrects a variable name in a Python code example within the sequential onloading documentation and adds a warning regarding the use of highly granular sequential targets (such as "Linear") with NVFP4/NVFP4A16 quantization. The reviewer suggested rephrasing the warning message to improve clarity and readability.

Comment on lines +38 to +39
!!! warning
Using a highly granular sequential target such as `"Linear"` is not supported for NVFP4/NVFP4A16 quantization, which requires that fused weights (qkv, gate_up, etc.) all be a part of the same sequential target.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The warning message can be rephrased to improve readability and clarity. Splitting the long sentence and using more formal terms for the projections makes the documentation easier to follow.

Suggested change
!!! warning
Using a highly granular sequential target such as `"Linear"` is not supported for NVFP4/NVFP4A16 quantization, which requires that fused weights (qkv, gate_up, etc.) all be a part of the same sequential target.
!!! warning
Using a highly granular sequential target such as "Linear" is not supported for NVFP4/NVFP4A16 quantization. This format requires that fused weights (such as QKV and gate/up projections) are kept within the same sequential target.

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.

Isn't this what #2732 resolves?

@mergify mergify Bot added the documentation Improvements or additions to documentation label May 29, 2026
@mergify

mergify Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🔴 1 of 1 protections blocking · waiting on 👀 reviews

Protection Waiting on
🔴 Require one maintainer review 👀 reviews

🔴 Require one maintainer review

Waiting for any of

  • approved-reviews-by=HDCharles
  • approved-reviews-by=brian-dellabetta
  • approved-reviews-by=dsikka
  • approved-reviews-by=kylesayrs
  • approved-reviews-by=yiliu30
This rule is failing.

All PRs must have at least one approving review from a maintainer before merging.

  • any of:
    • approved-reviews-by=HDCharles
    • approved-reviews-by=brian-dellabetta
    • approved-reviews-by=dsikka
    • approved-reviews-by=kylesayrs
    • approved-reviews-by=yiliu30
  • #changes-requested-reviews-by = 0

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

Labels

documentation Improvements or additions to documentation ready When a PR is ready for full CI testing before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants