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

Add check to ensure that there is enough room in permuted_indices #3403

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zimin2000
Copy link

Summary:
In scope of the MTIA IG Sprint we encountered a crash (T208229934) that turned out to be an incorrectly provided argument for permuted_lengths_sum argument.

This argument suppose to speed the operation, but actually, the true value it suppose to substitute is computed every time anyway.

One radical solution would be to drop the argument entirely, but it requires more thoughtful analysis. This diff just prevent clearly faulty case of allocated permuted_indices being less than required by the function logic.

Exactly this case was spotted in case of T208229934.

Differential Revision: D66274522

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66274522

Copy link

netlify bot commented Nov 22, 2024

Deploy Preview for pytorch-fbgemm-docs ready!

Name Link
🔨 Latest commit bbe82f8
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/6746726332e1c400086c5860
😎 Deploy Preview https://deploy-preview-3403--pytorch-fbgemm-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

zimin2000 pushed a commit to zimin2000/FBGEMM that referenced this pull request Nov 22, 2024
…torch#3403)

Summary:

In scope of the MTIA IG Sprint we encountered a crash (T208229934) that turned out to be an incorrectly provided argument for `permuted_lengths_sum` argument.


This argument suppose to speed the operation, but actually, the true value it suppose to substitute is computed every time anyway. 

One radical solution would be to drop the argument entirely, but it requires more thoughtful analysis. This diff just prevent clearly faulty case of allocated `permuted_indices` being less than required by the function logic.

Exactly this case was spotted in case of T208229934.

Differential Revision: D66274522
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66274522

zimin2000 pushed a commit to zimin2000/FBGEMM that referenced this pull request Nov 22, 2024
…torch#3403)

Summary:

In scope of the MTIA IG Sprint we encountered a crash (T208229934) that turned out to be an incorrectly provided argument for `permuted_lengths_sum` argument.


This argument suppose to speed the operation, but actually, the true value it suppose to substitute is computed every time anyway. 

One radical solution would be to drop the argument entirely, but it requires more thoughtful analysis. This diff just prevent clearly faulty case of allocated `permuted_indices` being less than required by the function logic.

Exactly this case was spotted in case of T208229934.

Differential Revision: D66274522
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66274522

Sergey Zimin added 2 commits November 26, 2024 17:13
Differential Revision: D66012567
…torch#3403)

Summary:
X-link: facebookresearch/FBGEMM#493


In scope of the MTIA IG Sprint we encountered a crash (T208229934) that turned out to be an incorrectly provided argument for `permuted_lengths_sum` argument.


This argument suppose to speed the operation, but actually, the true value it suppose to substitute is computed every time anyway. 

One radical solution would be to drop the argument entirely, but it requires more thoughtful analysis. This diff just prevent clearly faulty case of allocated `permuted_indices` being less than required by the function logic.

Exactly this case was spotted in case of T208229934.

Differential Revision: D66274522
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66274522

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

Successfully merging this pull request may close these issues.

2 participants