Repository navigation
fix(quantization): honor fp8 emulate for rowwise recipes - #4178
Conversation
Signed-off-by: Mohammed Alkindi <[email protected]>
|
/review |
|
/ok to test 2d0467e |
Signed-off-by: HuiyingLi <[email protected]>
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.
Reset Dynamo before the fixed-limit CPU dispatch check so prior scalar specializations cannot select a stale clamp bound or exhaust the compilation cache. Also compare against the independent eager reference with the existing exact tolerance. Signed-off-by: HuiyingLi <[email protected]>
|
/ok to test 46a76db |
HuiyingLi
left a comment
There was a problem hiding this comment.
Suggestion: Approve
Reviewed all three changed files at 46a76dbff1bfd662591ac53f355edf6d5f01ff25, including shared callers, TorchAO's frozen recipe configuration, and the SwiGLU test-isolation change. No actionable findings. The existing recipe factory plus dataclasses.replace preserves rowwise scaling and high-precision weight-gradient settings while honoring emulate=True; no additional abstraction is needed.
Validation:
- Reused verified results for byte-identical FP8 code: 25 native CPU tests passed, 3 skipped; the defective-source control fails exactly the two affected rowwise cases. All 35 supplemental CPU checks passed, covering configuration/filtering, FP32/BF16 forward/backward and optimizer updates, plus independent high-precision gradient oracles with wrong-recipe controls.
- The committed SwiGLU test candidate passed 36 focused native checks, with 11 CUDA-only skips. Controlled compiler-history replay fails before the isolation change and passes afterward; this does not claim to fix the pre-existing compiler's multi-limit behavior.
- Current CI L0: CPU 17,379 passed/1,316 skipped; GPU shards 9,289 passed/62 skipped and 9,311 passed/35 skipped; kernel parity 15 passed. All four jobs succeeded and consumed the verified built image. CI tested merge
4aaba1bfwith current main; all three reviewed files match the PR head exactly.
Earlier full local runs had 32 failures under a different dependency profile and are not counted as green suites. An unchanged DTensor test emitted unhandled threaded-backend errors; it is not counted as distributed-validation evidence. No FP8 hardware, distributed FP8, full-model, or checkpoint-loading validation is claimed. L2 jobs are still pending/running at submission.
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.