Skip to content
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

Check Tensor.dims equals the rank of the corresponding TensorView. #3117

Open
wujingyue opened this issue Oct 5, 2024 · 2 comments · Fixed by #3195
Open

Check Tensor.dims equals the rank of the corresponding TensorView. #3117

wujingyue opened this issue Oct 5, 2024 · 2 comments · Fixed by #3195
Assignees

Comments

@wujingyue
Copy link
Collaborator

wujingyue commented Oct 5, 2024

This is a follow-up to #3114. A bug could have been surfaced earlier if we had checked the equality.

To compare a Tensor and its TensorView, you'd have to construct the cpp Fusion by calling fd.execute. Then, for each tensor, get its TensorView via getFusionState(size_t index), then compare their dimensions.

Originally posted by @rdspring1 in #3114 (review)

wujingyue added a commit that referenced this issue Oct 15, 2024
Could have been caught if #3117 were fixed.
wujingyue added a commit that referenced this issue Oct 15, 2024
Could have been caught if #3117 were fixed.
wujingyue added a commit that referenced this issue Oct 15, 2024
Could have been caught if #3117 were fixed.
wujingyue added a commit that referenced this issue Oct 16, 2024
Could have been caught if #3117 were fixed.
wujingyue pushed a commit that referenced this issue Oct 17, 2024
Fixes #3117 

This PR creates `verifyTensorDimensions` to check that `Tensor.dims`
equals the rank of the corresponding `TensorView`. It is called in
`FusionDefinition::finalizeDefinition` after `buildFusionIr`.
@wujingyue
Copy link
Collaborator Author

Is it possible to check while building the fusion IR? Most errors (shown as exceptions) I encountered happen during

buildFusionIr(preschedFusion());
; they don't even reach verifyTensorDimensions.

(Sorry, I should have noticed the difference earlier)

@wujingyue
Copy link
Collaborator Author

Regardless of my last comment, the verification didn't seem to kick in for the following example.

git checkout wjy/3117
_bn && NVFUSER_DUMP=fusion_ir_original pytest tests/python/test_matmul.py -s -k 2d_x_3d
Tensor(index=2, ndim=2)
Inputs:
  T0_g_float[ iS0{2}, iS1{3} ]
  T1_g_float[ iS2{7}, iS3{3}, iS4{5} ]
Outputs:
  T2_g_float[ iS5{7}, iS6{2}, iS7{5}, rS8{3} ]

%kernel_math {
T2_g_float[ iS5{7}, iS6{2}, iS7{5}, rS8{3} ]
   = matmul(T0_g_float[ iS0{2}, iS1{3} ],
            T1_g_float[ iS2{7}, iS3{3}, iS4{5} ])
} // %kernel_math

Tensor(index=2, ndim=2)

c is 2D and T2 is 3D. However, verification didn't flag that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants