Refactor GroupedLinear quantization dispatch - #3326
Conversation
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Greptile SummaryThe PR centralizes GroupedLinear split-quantization and bias-gradient dispatch in a dedicated helper module while limiting grouped-quantizer validation to CustomRecipe generations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["GroupedLinear forward/backward"] --> B["_split_quantization._split_quantize"]
B --> C{"Quantized output requested?"}
C -- No --> D["Cast and torch.split"]
C -- Yes --> E{"Compute dbias with supported native quantizer?"}
E -- Yes --> F["tex.bgrad_quantize per split"]
E -- No --> G{"Expert 0 quantizer type"}
G -- Debug --> H["DebugQuantizer.multi_tensor_quantize"]
G -- Hybrid --> I["Split rowwise and columnwise sub-quantizers"]
G -- Native --> J["tex.split_quantize"]
G -- Identity or custom --> K["Views or Python quantizer fallback"]
Reviews (4): Last reviewed commit: "fix lint" | Re-trigger Greptile |
|
/te-ci pytorch L1 |
zhongbozhu
left a comment
There was a problem hiding this comment.
overall LGTM, just have some questions
| ) | ||
| return outputs, dbiases | ||
|
|
||
| concrete_quantizers = cast(Sequence[Quantizer], quantizers) |
There was a problem hiding this comment.
why do we need this cast(Sequence[Quantizer], quantizers)? feels like some extra CPU overhead?
There was a problem hiding this comment.
This is only for the type checker, like assume there are no None elements. At runtime, it returns the same object unchanged, it does not iterate through.
|
/te-ci pytorch |
Signed-off-by: Evgeny <etsykunov@nvidia.com>
for more information, see https://pre-commit.ci
|
/te-ci pytorch |
vthumbe1503
left a comment
There was a problem hiding this comment.
LGTM. We can rerun CI after fixing merge conflicts
Signed-off-by: Evgeny <etsykunov@nvidia.com>
Signed-off-by: Evgeny <etsykunov@nvidia.com>
|
/te-ci pytorch |
|
@vthumbe1503 ci is green |
| # a failed generation remains installed when the caller catches the error. | ||
| recipe = FP8GlobalStateManager.get_fp8_recipe() | ||
| self._validate_custom_recipe_quantizers(True, recipe) | ||
| if torch.is_grad_enabled(): |
There was a problem hiding this comment.
@negvet Had missed this in earlier review. But could we please move the is_grad_enabled inside the validate function to reduce CPU overheads in cases of non-custom recipes? Can be a seperate PR as well
There was a problem hiding this comment.
Sure, will do in a separate PR
Description
Move grouped quantization helpers into a dedicated module and replace separate split APIs with one. Keeps quantizer validation at construction only for CustomRecipe while skipping validation for the built-in recipes.
Followup for #2817
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: