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

Cleanup sparse tests #208

Merged
merged 3 commits into from
Feb 7, 2022
Merged

Cleanup sparse tests #208

merged 3 commits into from
Feb 7, 2022

Conversation

fmassa
Copy link
Contributor

@fmassa fmassa commented Feb 6, 2022

What does this PR do?

With #202 merged, we can now have a single entry-point for testing that sparse tensors behave as expected, with a unified set of tests to cover the required operators.

I've removed test_sparse_csr.py as it is now redundant compared to test_sparse_tensors.py.

I think a next step would be to clean test_custom_ops.py so that they test the bare minimum and doesn't depend on the legacy SparseCS tensor.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 6, 2022
@fmassa
Copy link
Contributor Author

fmassa commented Feb 6, 2022

Looks like doc failures are unrelated?

@blefaudeux
Copy link
Contributor

Looks like doc failures are unrelated?

Doc failure now fixed :) unrelated indeed

Comment on lines +117 to 118
def values(self):
return self.__values
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now unify obtaining the values of the sparse tensor under the same API, which follows the API from PyTorch

@fmassa
Copy link
Contributor Author

fmassa commented Feb 7, 2022

Merging, as this is a harmless PR wrt functionality.

@fmassa fmassa merged commit b160b05 into main Feb 7, 2022
@fmassa fmassa deleted the cleanup-sparse-tests branch February 7, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants