Repository navigation
fix(quantization): honor fp8 emulate for rowwise recipes - #4178
MohammedAlkindi wants to merge 2 commits into
Conversation
Signed-off-by: Mohammed Alkindi <alkndymhmd692@gmail.com>
|
/review |
|
/ok to test 2d0467e |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
No high-confidence findings. Reviewed the FP8 recipe configuration change and regression tests, including the directly affected caller. The emulation flag now reaches both the hardware check and conversion configuration; non-emulated recipes retain rejection below SM89.
Static review only; tests were not executed.
Review generated by codex
HuiyingLi
left a comment
There was a problem hiding this comment.
LGTM. replace(torchao_config, emulate=True) is the right way to carry the flag past from_recipe_name; unit tests pass locally (28 passed). The earlier CI failures were the install-test numpy/pyarrow issue fixed on main and an unrelated runtime-budget overrun in test_ddp_chunked_ce_two_rank_parity; re-triggered via proxy #4276.
Signed-off-by: HuiyingLi <willwin.lee@gmail.com>
18616c3
|
/ok to test 18616c3 |
HuiyingLi
left a comment
There was a problem hiding this comment.
Reviewed the complete three-file change at 18616c333c87edb63800c57874ed1210fe535b19, its callers, the recipe/config contract, and the actual imported TorchAO implementation. No blocking correctness issues found. The emulation override reaches all three GEMM configurations while preserving each recipe's scaling and backward settings, parameter identity, filtering, config precedence, and default hardware checks.
Validation: the native FP8 suite passed 25 tests, with 3 CUDA-gated skips. Against the defective implementation, exactly the two rowwise emulation regressions failed; the other 23 tests passed. An independently designed supplemental CPU suite passed 35 tests, including all recipe fields and call paths, FP32/BF16 forward/backward and one SGD update on 2D/3D inputs. All 16 comparisons with direct TorchAO conversion were exact; that reference shares TorchAO's numerical implementation. Separately, the high-precision weight-gradient recipe exactly matched the mathematical weight/bias-gradient oracle, while deliberately selecting ordinary rowwise failed that oracle.
These executed CPU results are reused from 2d0467e2dc566afcc51a12358ae49f7ab4d41310: its production and test sources are byte-identical to this head, whose sole additional change is the CI constraint. The recorded runtime was PyTorch 2.14.0a0+4fdf77b940.nv26.08 and TorchAO 0.17.0+git02105d4, with CUDA hidden. No hardware-FP8, GPU, compiled, distributed, checkpoint-loading, full-model, or performance coverage is claimed.
The installation constraint is the exact trusted-main PyArrow cap; YAML/shell validation and a fresh NumPy 1.26.4 / PyArrow 25.0.1 import passed. CI was triggered for this exact head: Installation Test is in progress and CICD NeMo is waiting. Those runs are not yet green.
What does this PR do ?
For the
rowwiseandrowwise_with_gw_hprecipes,apply_fp8_to_modelbuiltFloat8LinearConfig.from_recipe_name(...), which does not takeemulate, sofp8.emulate: truewas dropped. Below SM89 the call then raised "FP8 is only supported on SM89 or later GPUs ... pass emulate=True", even though emulate was set. The user's flag is now carried onto the recipe config.Changelog
emulate=Trueon the recipe-built config (viadataclasses.replace) whenfp8.emulateis true. On SM89+ this makes those recipes emulate when asked to, instead of running real FP8.Before your PR is "Ready for review"
Pre checks:
If you haven't finished some of the above items you can still open "Draft" PR.
Additional Information
test_fp8.py: 25 passed, 3 skipped; the two new emulate cases raise thatValueErroronmain. AI-assisted; I ran the file on 2d0467e.