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 torch.sparse overview section #85265

Closed
wants to merge 24 commits into from
Closed

Conversation

cpuhrsch
Copy link
Contributor

@cpuhrsch cpuhrsch commented Sep 19, 2022

The goal of this section is to provide a general overview of how PyTorch handles sparsity for readers who are already familiar with sparse matrices and their operators.

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 19, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/85265

Note: Links to docs will display an error until the docs builds have been completed.

❌ 4 Failures, 7 Pending

As of commit c71acbe:

The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@cpuhrsch cpuhrsch changed the title Sparsedocs1 [DO NOT REVIEW] Add torch.sparse overview section Sep 19, 2022
is in turn also backed and powered by sparse storage formats and kernels.

Also note that, for now, the user doesn't have a choice of the output layout. For example,
adding a sparse Tensor to a regular strided Tensor results in a sparse Tensor.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
adding a sparse Tensor to a regular strided Tensor results in a sparse Tensor.
adding a sparse Tensor to a regular strided Tensor results in a strided Tensor.

Copy link
Contributor

@bhosmer bhosmer left a comment

Choose a reason for hiding this comment

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

Looks really good! Couple quick editorial comments (not typos etc), will come back for another pass 😁

docs/source/sparse.rst Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
@cpuhrsch cpuhrsch marked this pull request as ready for review October 3, 2022 21:17
@cpuhrsch cpuhrsch changed the title [DO NOT REVIEW] Add torch.sparse overview section Add torch.sparse overview section Oct 3, 2022
@cpuhrsch cpuhrsch requested a review from pearu October 3, 2022 21:22
@facebook-github-bot
Copy link
Contributor

/easycla

As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details.

This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 4, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

Unary functions
---------------

All zero-preserving unary functions support sparse
Copy link
Collaborator

Choose a reason for hiding this comment

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

All zero-preserving unary functions listed below support sparse...

or possibly

All zero-preserving unary functions will support sparse...

Some zero-preserving unary functions, such as reLU, are not in fact supported, but the goal is to make sure they all are, so we might want the wording here to reflect that any missing zero-preserving unary function support is a reportable bug.

I also do not think this list is completely correct: torch.angle does not appear to support COO. Also all layouts support torch.expm1 which is missing.

Copy link
Contributor Author

@cpuhrsch cpuhrsch Oct 7, 2022

Choose a reason for hiding this comment

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

I'll update the wording to say that all zero preserving unary functions can be supported and encourage the reader to open an issue if they find one that's missing. I think it's fair to say that they're pretty easily supported, so it's ok to encourage people to report it. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds great. It is easy to add them so we should encourage people to point out missing ones.

Copy link

@jisaacso jisaacso left a comment

Choose a reason for hiding this comment

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

lg, added a handful of minor points

docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
docs/source/sparse.rst Outdated Show resolved Hide resolved
Copy link

@jisaacso jisaacso left a comment

Choose a reason for hiding this comment

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

thanks for addressing prior feedback

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 18, 2022
@cpuhrsch
Copy link
Contributor Author

@pytorchbot merge -f "docs only"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions
Copy link

Hey @cpuhrsch.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@cpuhrsch cpuhrsch added release notes: sparse release notes category topic: docs topic category labels Oct 18, 2022
cpuhrsch added a commit to cpuhrsch/pytorch that referenced this pull request Oct 20, 2022
The goal of this section is to provide a general overview of how PyTorch handles sparsity for readers who are already familiar with sparse matrices and their operators.
Pull Request resolved: pytorch#85265
Approved by: https://github.com/jisaacso
atalman pushed a commit that referenced this pull request Oct 20, 2022
The goal of this section is to provide a general overview of how PyTorch handles sparsity for readers who are already familiar with sparse matrices and their operators.
Pull Request resolved: #85265
Approved by: https://github.com/jisaacso
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request cla signed Merged release notes: sparse release notes category topic: docs topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants