Uh oh!
There was an error while loading. Please reload this page.
[PyTorch][FP8 CS] Remove the unnecessary torch reciprocal op in fp8 current scaling code path - #1950
[PyTorch][FP8 CS] Remove the unnecessary torch reciprocal op in fp8 current scaling code path#1950zhongbozhu wants to merge 2 commits into
Conversation
Signed-off-by: zhongboz <zhongboz@nvidia.com>
zhongbozhu
commented
Jul 14, 2025
/te-ci pytorch L1 |
yaox12
commented
Jul 15, 2025
/te-ci pytorch L1 |
BestJuly
commented
Jul 15, 2025
LGTM. |
timmoon10
left a comment
There was a problem hiding this comment.
Since we're no longer respecting the rowwise_data arg, we should error out if it is ever used:
NVTE_CHECK(!rowwise_data.has_value(), "Float8CurrentScalingQuantizer does not support creating tensor with pre-initialized buffer");#1952 is a systematic attempt to remove rowwise_data, but it's quite large. This PR will be much easier to merge.
@timmoon10 Seems like this change will break test_fusible_ops.py, do you know about the reason? |
The root cause is because the activation backward pass doesn't support FP8 current scaling: |
zhongbozhu
commented
Jul 16, 2025
Thanks for the root causing! I don't think layernorm_mlp will have this issue because the place where we call the dact function, the quantizer we gave is None here => so it's actually a None quantizer, calculating dact for the high precision, and then we call this line to do the quantization again here. So yes, the fusion is not supported, but layernorm_mlp has already taken that into account, in an implicit way of course. |
zhongbozhu
commented
Jul 21, 2025
@timmoon10 has the numerical issue in fused_op side been resolved? If it's already done, I can rebase and run the CI again. |
timmoon10
commented
Jul 22, 2025
#1952 is a comprehensive fix that removes the |
zhongbozhu
commented
Jul 22, 2025
Update: Will close this PR once #1952 is merged, which is a superset of this fix. |
Description
at::reciprocalwas part of FP8 current scaling recipe to make sure TPGEMM overlap works with FP8 current scaling. That's because the scales are not copied into user buffer for communication, but the old TPGEMM overlap codebase relies on thecreate_tensorAPI to construct the output tensor which requires a scaling factor tensor in it.Now that dependency has been removed, we no longer need this op.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: