-
Notifications
You must be signed in to change notification settings - Fork 636
Fix nvfp4 convert_and_update_tensor shape check #2670
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: 乙划 <zht108229@antgroup.com>
for more information, see https://pre-commit.ci
Greptile OverviewGreptile SummaryFixes shape validation for NVFP4 tensors when both rowwise and columnwise data are present. The issue occurred because columnwise data enforces 2D shapes while rowwise data can be 3D or higher, causing the direct shape comparison to fail. The fix:
This properly handles cases where rowwise data has 3D+ shapes while maintaining shape consistency validation. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant convert_and_update_tensor
participant convert_shape_back_from_fp4
participant compressShapeTo2D
Caller->>convert_and_update_tensor: NVFP4Tensor with rowwise & columnwise data
alt columnwise_data exists
convert_and_update_tensor->>convert_shape_back_from_fp4: getTensorShape(*columnwise_data), transpose=true
convert_shape_back_from_fp4-->>convert_and_update_tensor: shape (2D enforced)
alt rowwise_data also exists
convert_and_update_tensor->>convert_shape_back_from_fp4: getTensorShape(*rowwise_data), transpose=false
convert_shape_back_from_fp4-->>convert_and_update_tensor: expected_shape (could be 3D+)
convert_and_update_tensor->>compressShapeTo2D: expected_shape
compressShapeTo2D-->>convert_and_update_tensor: expected_shape_2d
convert_and_update_tensor->>compressShapeTo2D: shape
compressShapeTo2D-->>convert_and_update_tensor: shape_2d
convert_and_update_tensor->>convert_and_update_tensor: Validate shape_2d == expected_shape_2d
convert_and_update_tensor->>convert_and_update_tensor: Use expected_shape as ground truth
end
else only rowwise_data exists
convert_and_update_tensor->>convert_shape_back_from_fp4: getTensorShape(*rowwise_data), transpose=false
convert_shape_back_from_fp4-->>convert_and_update_tensor: shape
end
convert_and_update_tensor-->>Caller: TensorWrapper with correct shape
|
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.
1 file reviewed, 2 comments
Additional Comments (2)
|
|
@skydoorkai I think the comments from Greptile make sense (even though they are not very high priority). Could you fix those? Other than that LGTM. |
Signed-off-by: 乙划 <zht108229@antgroup.com>
for more information, see https://pre-commit.ci
Updated according to Greptile comments to add headers and compare 2D shapes. |
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.
1 file reviewed, 1 comment
|
/te-ci pytorch |
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.
1 file reviewed, no comments
Description
This is to fix #2607
For nvfp4's columnwise data , it is using enforced 2D shape. Thus, the original check would fail if rowwise_data shape is 3D shape.
To fix :
(1) expected_data should be enforced into 2D shape from rowwise_data's shape.
(2) use rowwise_data's shape as the “ground truth" shape.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: