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

Fix fbgemm::permute_1D_sparse_data's aliasing behavior #2180

Closed
wants to merge 1 commit into from

Conversation

zou3519
Copy link
Contributor

@zou3519 zou3519 commented Nov 30, 2023

Summary:
The schema for this operator promises that it is functional (that is, it does
not return output Tensors that are views on the inputs). There is a fast-path
that does return views on the input. We change it to return clones instead of
views. fbgemm::permute_2d_sparse_data already has this fix; looks like we just
didn't apply it to fbgemm::permute_1d_sparse_data

Differential Revision: D51711257

Copy link

netlify bot commented Nov 30, 2023

Deploy Preview for pytorch-fbgemm-docs canceled.

Name Link
🔨 Latest commit 4b9fe96
🔍 Latest deploy log https://app.netlify.com/sites/pytorch-fbgemm-docs/deploys/6568c452f3128800099d8fbf

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

…iant (pytorch#2180)

Summary:

- We fix fbgemm::permute_1d_sparse_data's implementation to never return
  views. The schema for the operator promises that it is functional. This was
  causing flaky tests.
- fbgemm::permute_2d_sparse_data already has that fix applied, so we just
  remove the skip

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

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

@williamwen42 williamwen42 self-requested a review November 30, 2023 18:41
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 1c93072.

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.

3 participants