Skip to content

Roderick wu/sanity check fouroversix - #3015

Draft
Roderick-Wu wants to merge 13 commits into
mainfrom
Roderick-Wu/sanity-check-fouroversix
Draft

Roderick wu/sanity check fouroversix#3015
Roderick-Wu wants to merge 13 commits into
mainfrom
Roderick-Wu/sanity-check-fouroversix

Conversation

@Roderick-Wu

Copy link
Copy Markdown
Collaborator

for diffing

Roderick Wu and others added 12 commits July 23, 2026 11:50
- Thread global_scale through _calculate_error for correct NVFP4
  two-level quantization error evaluation
- Add expand parameter for wider-than-observed grid search ranges
- Add global_scale_max observer_kwargs for custom FP8 scale cap
- Add _CustomFP8ScaleData and global_scale_max support in get_global_scale

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Roderick Wu <Roderick-Wu@dgx-b200-02.mgmt.accl-001.lab.rdu2.dc.redhat.com>
Co-authored-by: Brian Dellabetta <brian-dellabetta@users.noreply.github.com>
Signed-off-by: Roderick Wu <roderickwu2003@gmail.com>
Signed-off-by: Roderick Wu <Roderick-Wu@h100-01.nemg-001.lab.rdu2.dc.redhat.com>
Signed-off-by: Roderick Wu <Roderick-Wu@ip-172-31-44-201.us-west-2.compute.internal>
Signed-off-by: Roderick Wu <Roderick-Wu@ip-172-31-44-201.us-west-2.compute.internal>
Signed-off-by: Roderick Wu <Roderick-Wu@ip-172-31-44-201.us-west-2.compute.internal>
Signed-off-by: Roderick Wu <Roderick-Wu@ip-172-31-44-201.us-west-2.compute.internal>
- Register name: nvfp4_mse → nvfp4_expanded_mse
- maxshrink: 0.56 → 1 - 0.8/1.8 (sweeps 1.8x→0.8x)
- grid: 100 → 200 (finer search resolution)
- patience: 5 → 1000 (full range sweep)
- Fix docstring to reference correct observer name

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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: 2a558b08-9205-4fac-9f45-97eed569fbe3

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

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.

@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The quality checks have failed. Please run make style and make quality under
the root directory to adddress the lint failures. You will need to install the
dev optional install to get the required linting packages:
https://github.com/vllm-project/llm-compressor/blob/main/CONTRIBUTING.md

@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 introduces the FourOverSixObserver for adaptive block scaling in NVFP4 quantization, which selects between M=6 and M=4 scales per block to minimize quantization error. Additionally, it updates the MSE observer and grid search utilities to support range expansion and global scale priors, and registers a new NVFP4ExpandedMSEObserver. The feedback suggests refactoring FourOverSixObserver to override the base class's get_global_scale method, which simplifies get_qparams and avoids duplicating global scale computation logic.

Comment on lines +81 to +111
@torch.no_grad
def get_qparams(self) -> QParamsDict:
assert (
self.has_statistics
), "No statistics available. Call observer(value) first."

global_scale = None

if self.args.strategy == QuantizationStrategy.TENSOR_GROUP:
all_stats = self.fusion_handler.get_fused_statistics()
global_absmax = all_stats[0]["max_vals"].max()
for stats in all_stats:
global_absmax = torch.max(
global_absmax, -stats["min_vals"].min()
)
global_absmax = torch.max(
global_absmax, stats["max_vals"].max()
)

global_scale = generate_gparam(
-global_absmax.reshape(1),
global_absmax.reshape(1),
scale_data=_FP8ScaleData256,
)

scale_6, zero_point = calculate_qparams(
min_vals=self.min_vals,
max_vals=self.max_vals,
quantization_args=self.args,
global_scale=global_scale,
)

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 logic for computing the global scale in get_qparams duplicates the structure of get_global_scale from the base Observer class, except for the custom scale_data parameter. Overriding get_global_scale directly in FourOverSixObserver simplifies get_qparams and aligns with the base class design.

    def get_global_scale(self) -> torch.Tensor | None:
        if self.args.strategy != QuantizationStrategy.TENSOR_GROUP:
            return None

        all_stats = self.fusion_handler.get_fused_statistics()
        global_absmax = all_stats[0]["max_vals"].max()
        for stats in all_stats:
            global_absmax = torch.max(
                global_absmax, -stats["min_vals"].min()
            )
            global_absmax = torch.max(
                global_absmax, stats["max_vals"].max()
            )

        return generate_gparam(
            -global_absmax.reshape(1),
            global_absmax.reshape(1),
            scale_data=_FP8ScaleData256,
        )

    @torch.no_grad
    def get_qparams(self) -> QParamsDict:
        assert (
            self.has_statistics
        ), "No statistics available. Call observer(value) first."

        global_scale = self.get_global_scale()

        scale_6, zero_point = calculate_qparams(
            min_vals=self.min_vals,
            max_vals=self.max_vals,
            quantization_args=self.args,
            global_scale=global_scale,
        )

@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.

@mergify mergify Bot removed the quality-failed label Aug 10, 2026
@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

The quality checks have failed. Please run make style and make quality under
the root directory to adddress the lint failures. You will need to install the
dev optional install to get the required linting packages:
https://github.com/vllm-project/llm-compressor/blob/main/CONTRIBUTING.md

@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

@kylesayrs kylesayrs mentioned this pull request Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant