-
Notifications
You must be signed in to change notification settings - Fork 24.7k
[MPS] Fix sliced cast #138314
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
[MPS] Fix sliced cast #138314
Conversation
This fixes internal crash due to the invalid bufer size computation if sliced API is used Not sure what was the purpose of ```c++ IntArrayRef baseShape; if (src.is_view()) { baseShape = src._base().sizes(); } else { baseShape = getIMPSAllocator()->getBufferShape(src.storage().data()); } int flattenedShaped = 1; for (const auto i : c10::irange(baseShape.size())) { flattenedShaped *= baseShape[i]; } ``` As flattenShaped could be much easier computed as `[srcBuf lengh]/src.element_size()` When someone allocated buffer to hold say uint8 and that view-casted it to float16, attempt to compute `baseShape` returned sizes of original tensor in its data type, rather than size in new dtypes Fixes #137800
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/138314
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 451e406 with merge base a9014d2 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
Sounds good thanks!
We can use storage().size() as well if we don't want to query the srcBuf.
I think the main idea was that the storage buffer might be bigger than the actual |
@pytorchbot merge -f "MPS changes are green" |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
@pytorchbot cherry-pick --onto release/2.5 -c regression |
This fixes internal crash due to the invalid bufer size computation if sliced API is used Not sure what was the purpose of ```c++ IntArrayRef baseShape; if (src.is_view()) { baseShape = src._base().sizes(); } else { baseShape = getIMPSAllocator()->getBufferShape(src.storage().data()); } int flattenedShaped = 1; for (const auto i : c10::irange(baseShape.size())) { flattenedShaped *= baseShape[i]; } ``` As flattenShaped could be much easier computed as `[srcBuf lengh]/src.element_size()`, and even if `srcBuf` is padded it's a safe thing to do. When someone allocated buffer to hold say uint8 and that view-casted it to float16, attempt to compute `baseShape` returned sizes of original tensor in its data type, rather than size in new dtypes Fixes #137800 Pull Request resolved: #138314 Approved by: https://github.com/albanD, https://github.com/DenisVieriu97 (cherry picked from commit de16159)
Cherry picking #138314The cherry pick PR is at #138535 and it is recommended to link a regression cherry pick PR with an issue. Details for Dev Infra teamRaised by workflow job |
[MPS] Fix sliced cast (#138314) This fixes internal crash due to the invalid bufer size computation if sliced API is used Not sure what was the purpose of ```c++ IntArrayRef baseShape; if (src.is_view()) { baseShape = src._base().sizes(); } else { baseShape = getIMPSAllocator()->getBufferShape(src.storage().data()); } int flattenedShaped = 1; for (const auto i : c10::irange(baseShape.size())) { flattenedShaped *= baseShape[i]; } ``` As flattenShaped could be much easier computed as `[srcBuf lengh]/src.element_size()`, and even if `srcBuf` is padded it's a safe thing to do. When someone allocated buffer to hold say uint8 and that view-casted it to float16, attempt to compute `baseShape` returned sizes of original tensor in its data type, rather than size in new dtypes Fixes #137800 Pull Request resolved: #138314 Approved by: https://github.com/albanD, https://github.com/DenisVieriu97 (cherry picked from commit de16159) Co-authored-by: Nikita Shulga <nikita.shulga@gmail.com>
This fixes internal crash due to the invalid bufer size computation if sliced API is used
Not sure what was the purpose of
As flattenShaped could be much easier computed as
[srcBuf lengh]/src.element_size()
, and even ifsrcBuf
is padded it's a safe thing to do.When someone allocated buffer to hold say uint8 and that view-casted it
to float16, attempt to compute
baseShape
returned sizes of originaltensor in its data type, rather than size in new dtypes
Fixes #137800