Repository navigation
Enable TE custom quantization recipe #2005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
48e3b48
Enable custom recipe in TE
negvet cdab16f
Merge main
negvet f81eb9f
Support fp4 recipe to be custom as well as fp8
negvet 36dbfbf
Merge branch 'main' into etsykunov/custom_quant_recipe
negvet bd21b45
Merge branch 'main' into etsykunov/custom_quant_recipe
ko3n1g 64a5a52
Fix linters
negvet 2245c22
Move validations to transformer_config post_init
9c29ab1
Merge branch 'main' into etsykunov/custom_quant_recipe
negvet 43e01a3
Merge branch 'main' into etsykunov/custom_quant_recipe
negvet File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,14 +2,15 @@ | |
|
|
||
| """Utility functions related to FP8 that are used throughout Megatron core""" | ||
|
|
||
| import importlib | ||
| import weakref | ||
| from contextlib import nullcontext | ||
| from functools import wraps | ||
| from typing import List, Optional | ||
| from typing import List, Optional, Union | ||
|
|
||
| import torch | ||
|
|
||
| from megatron.core.enums import Fp8Recipe | ||
| from megatron.core.enums import Fp4Recipe, Fp8Recipe | ||
| from megatron.core.transformer.transformer_config import TransformerConfig | ||
| from megatron.core.utils import get_te_version, is_te_min_version | ||
|
|
||
|
|
@@ -104,6 +105,53 @@ def dequantize_fp8_tensor(fp8_tensor: torch.Tensor) -> torch.Tensor: | |
| return fp8_tensor.from_float8() | ||
|
|
||
|
|
||
| def _resolve_callable_from_python_import_path(dotted_path: str): | ||
| """Resolve a Python import path like 'pkg.mod.func' to a callable. | ||
|
|
||
| Raises ValueError with clear message on failure. | ||
| """ | ||
| if not isinstance(dotted_path, str) or not dotted_path: | ||
| raise ValueError( | ||
| "fp8_quantizer_factory must be a non-empty string with format 'pkg.mod.func'." | ||
| ) | ||
|
|
||
| parts = dotted_path.rsplit(".", 1) | ||
| if len(parts) == 1: | ||
| raise ValueError(f"Invalid fp8_quantizer_factory '{dotted_path}'. Expected 'pkg.mod.func'.") | ||
| module_path, attr = parts[0], parts[1] | ||
|
|
||
| try: | ||
| mod = importlib.import_module(module_path) | ||
| except Exception as exc: | ||
| raise ValueError( | ||
| f"Failed to import module '{module_path}' for fp8_quantizer_factory: {exc}" | ||
| ) from exc | ||
|
|
||
| fn = getattr(mod, attr, None) | ||
| if fn is None: | ||
| raise ValueError( | ||
| f"Attribute '{attr}' not found in module '{module_path}' for fp8_quantizer_factory." | ||
| ) | ||
| if not callable(fn): | ||
| raise ValueError( | ||
| f"Resolved attribute '{module_path}.{attr}' is not callable for fp8_quantizer_factory." | ||
| ) | ||
| return fn | ||
|
|
||
|
|
||
| def _get_custom_recipe(quantizer_factory_python_path: str) -> Union[Fp8Recipe, Fp4Recipe]: | ||
| quantizer_factory = _resolve_callable_from_python_import_path(quantizer_factory_python_path) | ||
| try: | ||
| custom_recipe = transformer_engine.common.recipe.CustomRecipe(qfactory=quantizer_factory) | ||
| except AttributeError: | ||
| raise ValueError( | ||
| """CustomRecipe recipe is not available in this version of | ||
| Transformer Engine. Please make sure you are using TE version | ||
| >= 2.9.0.dev0.""" | ||
| ) | ||
| return custom_recipe | ||
|
|
||
|
|
||
| def get_fp8_align_size(fp8_recipe: Fp8Recipe) -> int: | ||
| """Get the alignment size required for fp8 GEMM.""" | ||
| if fp8_recipe == Fp8Recipe.mxfp8: | ||
|
|
@@ -466,6 +514,14 @@ def get_fp8_recipe(config: TransformerConfig): | |
| fp8_recipe = transformer_engine.common.recipe.MXFP8BlockScaling( | ||
| fp8_format=fp8_format | ||
| ) | ||
| elif config.fp8_recipe == Fp8Recipe.custom: | ||
| if not config.fp8_quantizer_factory: | ||
| raise ValueError( | ||
| "Python import path, e.g. package.module.quantizer_factory, " | ||
| "must be provided via --fp8-quantizer-factory when " | ||
| "--fp8-recipe custom is selected." | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not add this validation in the transformer config
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, fixed in 2245c22 |
||
| fp8_recipe = _get_custom_recipe(config.fp8_quantizer_factory) | ||
| else: | ||
| raise ValueError( | ||
| "Float8CurrentScaling, MXFP8BlockScaling, Float8BlockwiseScaling and " | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these seem more reasonable to be put in TE. Not sure why the recipe and import checking need to be in mcore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mcore owns cli/config, flag parsing, UX errors etc.
_get_custom_recipeis an orchestration kind of thing.That is why I put it at the mcore side.
Import path resolution is similar, because import management fits mcore well.
TE just owns abstractions (
CustomRecipeetc.), being at a lower level of the stack.With that being said, I do not mind having a minor narrow helper utility function
transformer_engine.utils.resolve_path(path)in TE that can do some basic path resolution for the quantizer factory.But from my perspective, this is not strictly necessary (or can be added later), although I am neutral.