Roderick wu/sanity check fouroversix - #3015
Conversation
- 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>
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@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>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
The quality checks have failed. Please run |
There was a problem hiding this comment.
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.
| @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, | ||
| ) |
There was a problem hiding this comment.
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,
)|
👋 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. |
|
The quality checks have failed. Please run |
Merge Protections🔴 1 of 1 protections blocking · waiting on 👀 reviews
🔴 Require one maintainer reviewWaiting for any of
This rule is failing.All PRs must have at least one approving review from a maintainer before merging.
|
for diffing