Skip to content

sparse_mask: backward support for sparse lhs (take 2)#104341

Closed
nikitaved wants to merge 14 commits intogh/nikitaved/59/basefrom
gh/nikitaved/59/head
Closed

sparse_mask: backward support for sparse lhs (take 2)#104341
nikitaved wants to merge 14 commits intogh/nikitaved/59/basefrom
gh/nikitaved/59/head

Conversation

@nikitaved
Copy link
Copy Markdown
Collaborator

@nikitaved nikitaved commented Jun 28, 2023 •
edited
Loading

@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Jun 28, 2023 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

As of commit f27da78:
💚 Looks good so far! There are no failures yet. 💚

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

@pytorch-bot pytorch-bot bot added the release notes: sparse release notes category label Jun 28, 2023
This was referenced Jun 23, 2023
Closed
Closed
Closed
@nikitaved nikitaved requested a review from pearu June 28, 2023 10:29
@nikitaved nikitaved added module: sparse Related to torch.sparse module: autograd Related to torch.autograd, and the autograd engine in general ciflow/trunk Trigger trunk jobs on your pull request ciflow/slow labels Jun 28, 2023
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 28, 2023

Sorry to bother you again with this one, @pearu, but could you please approve this one again?

@nikitaved nikitaved added the topic: new features topic category label Jun 28, 2023
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 28, 2023

@albanD , @soulitzer , could you please also approve it for good measure since @cpuhrsch is on PTO?

nikitaved added 3 commits June 28, 2023 12:46
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
d97c6ea 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
a40acbc 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
d82a1af 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
albanD
albanD approved these changes Jun 28, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Looks good!

amjames
amjames reviewed Jun 28, 2023
View reviewed changes
pearu
pearu reviewed Jun 28, 2023
View reviewed changes
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
b90c8af 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
pearu
pearu approved these changes Jun 28, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@pearu pearu left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, @nikitaved!

amjames
amjames approved these changes Jun 28, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@amjames amjames left a comment

Choose a reason for hiding this comment

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

Thanks!

@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
923fe66 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 29, 2023 •
edited
Loading

Alright, it looks weird. Slowtests are failing in somewhat non-deterministic manner across shards. Is there a way to trigger these things locally? I have no issues when running gradcheck with fast_mode=False.

@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
7e61080 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@pearu
Copy link
Copy Markdown
Collaborator

pearu commented Jun 29, 2023

Alright, it looks weird. Slowtests are failing in somewhat non-deterministic manner across shards. Is there a way to trigger these things locally? I have no issues when running gradcheck with fast_mode=False.

Yes, try defining env variable PYTORCH_TEST_WITH_SLOW_GRADCHECK=1 prior running the gradcheck tests.

@amjames amjames mentioned this pull request Jun 29, 2023
Closed
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 29, 2023 •
edited
Loading

Thank you, @pearu! Now it fails locally, but only when masked=True. It seems I know why...

@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
1998e5e 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 29, 2023

Nice, problem solved! One more for masked=True being very confusing ;)

nikitaved added 4 commits June 29, 2023 15:03
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
c100711 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
b50c0c3 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
38020ad 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
e51deb0 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 30, 2023

The failures are unrelated :) I think it is good to go!

@amjames
Copy link
Copy Markdown
Collaborator

amjames commented Jun 30, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jun 30, 2023

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jun 30, 2023

Merge failed

Reason: 1 jobs have failed, first few of them are: slow / linux-bionic-cuda12.1-py3-gcc9-slow-gradcheck / test (default, 1, 4, linux.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team Raised by workflow job

@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jun 30, 2023

@amjames , I will rebase, then we can merge :)

nikitaved added 2 commits June 30, 2023 14:12
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
9c5f7c3 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Update on "sparse_mask: backward support for sparse lhs (take 2)"
f27da78 
This is a copy of #95165 with some bug fixes.




cc alexsamardzic pearu cpuhrsch amjames bhosmer ezyang albanD zou3519 gqchen soulitzer Lezcano Varal7

[ghstack-poisoned]
@nikitaved
Copy link
Copy Markdown
Collaborator Author

nikitaved commented Jul 3, 2023

@pytorchbot merge

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

pytorchmergebot commented Jul 3, 2023

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

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

@facebook-github-bot facebook-github-bot deleted the gh/nikitaved/59/head branch July 6, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pearu pearu pearu approved these changes

@albanD albanD albanD approved these changes

@amjames amjames amjames approved these changes

@soulitzer soulitzer Awaiting requested review from soulitzer

Assignees

No one assigned

Labels

ciflow/slow ciflow/trunk Trigger trunk jobs on your pull request Merged module: autograd Related to torch.autograd, and the autograd engine in general module: sparse Related to torch.sparse open source release notes: sparse release notes category topic: new features topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nikitaved @pearu @amjames @pytorchmergebot @albanD @pytorchbot