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.

Sign up for GitHub

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

Jump to bottom

CUDA support in the CSR layout: sparse_to_dense/add_sparse_csr #59011

Closed
wants to merge 2 commits into from
Closed

CUDA support in the CSR layout: sparse_to_dense/add_sparse_csr #59011

wants to merge 2 commits into from

Conversation

aocsa
Copy link
Contributor

@aocsa aocsa commented May 26, 2021 •
edited

Stack from ghstack:

Restack of #57275, see review history there.

Differential Revision: D28719550

@aocsa aocsa requested a review from ezyang as a code owner May 26, 2021 18:41
This was referenced May 26, 2021
Closed
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 26, 2021 •
edited

💊 CI failures summary and remediations

As of commit dd0afbf (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@bhosmer
Copy link
Contributor

bhosmer commented May 26, 2021

@bhosmer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

bhosmer
bhosmer approved these changes May 26, 2021
View reviewed changes
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.

Transferring approval from #57275

@bhosmer bhosmer mentioned this pull request May 26, 2021
Closed
@aocsa
minor update/feedback comments addressed on "CUDA support in the CSR …
dd0afbf 
…layout: sparse_to_dense/add_sparse_csr"


Restack of #57275, see review history there.

Differential Revision: [D28719550](https://our.internmc.facebook.com/intern/diff/D28719550)

[ghstack-poisoned]
@aocsa
Copy link
Contributor Author

aocsa commented May 27, 2021 •
edited

Thanks for the feedback @ngimel, I've addressed the comments fixing where possible or giving referenced information. When issues are not only related to CSR I create new issues as detailed here:

Issue: support auto generation of device check for sparse tensors #59058
Issue: Add 64-bit indices support to csrmm2 #58899
Issue: Relaxing constraints to s_addmm_out_sparse_dense_cuda_worker #59099
Issue: Use of storage_offset is not needed in add_out_dense_sparse_csr_cuda #59101
Issue: Data access pattern in the loop in add_out_dense_sparse_csr_cuda could be pretty bad #59104

Hope there is not more new issues, If so unfortunately, my timing is tight, as I won't be able to work on this after this month. So make big changes will not be possible and I would prefer to create new issues and address them in future next PRs or figure out a handoff strategy. Thanks cc @bhosmer

@bhosmer
Copy link
Contributor

bhosmer commented May 27, 2021

@bhosmer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

This was referenced May 27, 2021
Open
Open
@facebook-github-bot
Copy link
Contributor

@bhosmer merged this pull request in 41054f2.

@aocsa aocsa mentioned this pull request May 31, 2021
Open
16 tasks
@facebook-github-bot facebook-github-bot deleted the gh/aocsa/31/head branch May 31, 2021 14:17
@rgommers rgommers added the module: sparse Related to torch.sparse label Jun 7, 2021
deniskokarev pushed a commit to deniskokarev/pytorch that referenced this pull request Jun 9, 2021
@aocsa @deniskokarev
CUDA support in the CSR layout: sparse_to_dense/add_sparse_csr (pytor…
d1f8775 
…ch#59011)

Summary: Pull Request resolved: pytorch#59011

Test Plan: Imported from OSS

Reviewed By: zou3519

Differential Revision: D28719550

Pulled By: bhosmer

fbshipit-source-id: 530c7cd1b20ae6d8865fd414afaf6fab27a643e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngimel ngimel ngimel left review comments

@bhosmer bhosmer bhosmer approved these changes

@ezyang ezyang Awaiting requested review from ezyang

Assignees
No one assigned
Labels
cla signed Merged module: sparse Related to torch.sparse open source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@aocsa @facebook-github-bot @bhosmer @ngimel @rgommers @pytorchbot