Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts #82122

amjames · 2022-07-25T16:21:59Z

Stack from ghstack (oldest at bottom):

sparse layouts [ghstack-poisoned]

facebook-github-bot · 2022-07-25T16:22:17Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/82122
✖️ Python docs build was skipped
✖️ C++ docs build was skipped
❓Need help or want to give feedback on the CI? Visit our office hours

❌ 1 New Failures

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

Expand to see more

1/1 failures introduced in this PR

🕵️‍♀️ 1 failure not recognized by patterns:

The following CI failures may be due to changes from the PR

Job	Step
^build	^Unknown

This comment was automatically generated by Dr. CI (expand for details).

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

Click here to manually regenerate this comment.

…compressed" sparse layouts [ghstack-poisoned]

… BSR/BSC layouts" [ghstack-poisoned]

amjames · 2022-07-29T19:45:52Z

aten/src/ATen/native/TensorShape.cpp

+    // the fact that the block should not be possible to hit this code block
+    TORCH_INTERNAL_ASSERT_DEBUG_ONLY(
+        false, "transpose(): Shouldn't have reached this point");
+    result_vals = AT_DISPATCH_PLAIN_SPARSE_COMPRESSED_LAYOUTS(


I left this code in although support is disabled and the block is unreachable.

Dense dimension support for CSR/CSC is in the immediate future, but I can stash the change locally and bring it back out when we are ready to actually add it if that is preferred.

… BSR/BSC layouts" [ghstack-poisoned]

bhosmer

This LGTM, couple super minor comments inline but really nice overall. Def worth getting @nikitaved's eyes on it too though ofc

bhosmer · 2022-08-03T21:09:46Z

aten/src/ATen/native/TensorShape.cpp

+    };
+    const auto dim1_type = classify_dim(dim1);
+    TORCH_CHECK(
+        classify_dim(dim1) == transpose_type,


s/classify_dim(dim1)/dim1_type?

aten/src/ATen/native/TensorShape.cpp

test/test_sparse_csr.py

… BSR/BSC layouts" [ghstack-poisoned]

aten/src/ATen/native/TensorShape.cpp

… BSR/BSC layouts" [ghstack-poisoned]

amjames · 2022-09-01T23:06:45Z

@pytorchbot merge

pytorchmergebot · 2022-09-01T23:08:13Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

…youts (#82122) Pull Request resolved: #82122 Approved by: https://github.com/bhosmer

pytorchmergebot · 2022-09-01T23:38:30Z

Merge failed

Reason: Failed to merge; some land checks failed: pull, pull / linux-bionic-py3_7-clang8-xla / test (xla, 1, 1, linux.2xlarge)

If you believe this is an error, you can use the old behavior with @pytorchbot merge -g (optionally with the ciflow/trunk to get land checks) or use @pytorchbot merge -f "some reason here". For more information, see the bot wiki.

Please reach out to the PyTorch DevX Team with feedback or questions!

Details for Dev Infra team

Raised by workflow job

amjames · 2022-09-02T15:14:05Z

@pytorchbot merge

pytorchmergebot · 2022-09-02T15:16:26Z

@pytorchbot successfully started a merge job. Check the current status here and land check progress here.
The merge job was triggered with the land checks (-l) flag. If you did not specify this flag yourself, you are likely enrolled in the land checks rollout. This means that your change will be merged once all checks on your PR and the land checks have passed (ETA 4 Hours). If you need to coordinate lands between different changes and cannot risk a land race, please add the ciflow/trunk label to your PR and wait for signal to complete, and then land your changes in proper order. Having trunk, pull, and Lint pre-run on a PR will bypass land checks and the ETA should be immediate. If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

…youts (#82122) Pull Request resolved: #82122 Approved by: https://github.com/bhosmer

github-actions · 2022-09-02T17:43:41Z

Hey @amjames.
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.

…youts (#82122) (#82122) Summary: Pull Request resolved: #82122 Approved by: https://github.com/bhosmer Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/9b115c7bd32b4a516f253a217bc8ec47bd07c44d Reviewed By: mehtanirav, izaitsevfb Differential Revision: D39277567 fbshipit-source-id: 9b7ae56319bba48becf8df1eb767683b156d81a3

cpuhrsch · 2022-09-08T22:41:05Z

aten/src/ATen/native/TensorShape.cpp

+
+  // We have validated everything, early exit for equal dims (no effect)
+  if (dim0 == dim1) {
+    return self.clone();


Isn't using clone here an issue? I thought transpose was a view. Shouldn't this return an alias? cc @bhosmer @amjames

amjames · 2022-09-09T14:03:57Z

test/test_sparse_csr.py

+            # correct layout
+            self.assertEqual(transpose.layout, subject.layout)
+            # transpose must be return a view
+            _check_transpose_view(subject, transpose)


@cpuhrsch re: self.clone() Why does this check not fail, if the result is not a view?

That's a good question. The test must not hit that branch or the check is too weak. I adopted it from existing view checks. Seems worthwhile investigating.

Sparse Compressed Transpose batch dim support for (block) compressed

d9676d0

sparse layouts [ghstack-poisoned]

amjames requested a review from a team as a code owner July 25, 2022 16:22

This was referenced Jul 25, 2022

Add support for select of batch dims for all sparse compressed formats. #82119

Closed

Fix BSR->Dense Batched Bug #82120

Closed

Support BSC->CSC conversion #82121

Closed

Dense <-> bsc conversions #80781

Closed

facebook-github-bot added the cla signed label Jul 25, 2022

pytorchbot added the open source label Jul 25, 2022

cpuhrsch requested a review from bhosmer July 25, 2022 16:50

amjames added 3 commits July 25, 2022 14:46

Update on "Sparse Compressed Transpose batch dim support for (block) …

ee769df

…compressed" sparse layouts [ghstack-poisoned]

Update on "Sparse Compressed Transpose batch dim support for (block) …

585cf2a

…compressed" sparse layouts [ghstack-poisoned]

Update on "Sparse Compressed Transpose batch dim support for (block) …

f7f3e66

…compressed" sparse layouts [ghstack-poisoned]

amjames mentioned this pull request Jul 26, 2022

Transpose of a sparse tensor is not a view operation #68699

Open

amjames changed the title ~~Sparse Compressed Transpose batch dim support for (block) compressed~~ Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts Jul 26, 2022

amjames added 4 commits July 26, 2022 14:45

Update on "Sparse Compressed Transpose add support for Batch dims and…

13f4207

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

fb1185f

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

f73b5c8

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

1ddfc9d

… BSR/BSC layouts" [ghstack-poisoned]

amjames added the module: sparse Related to torch.sparse label Jul 27, 2022

Update on "Sparse Compressed Transpose add support for Batch dims and…

42a6fb3

… BSR/BSC layouts" [ghstack-poisoned]

amjames commented Jul 29, 2022

View reviewed changes

amjames requested a review from nikitaved July 29, 2022 22:43

amjames added 6 commits July 29, 2022 18:23

Update on "Sparse Compressed Transpose add support for Batch dims and…

11554be

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

c0512ba

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

3a9c28e

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

e91b03e

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

d15e9ac

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

f0d6bba

… BSR/BSC layouts" [ghstack-poisoned]

bhosmer approved these changes Aug 3, 2022

View reviewed changes

amjames added 7 commits August 10, 2022 12:09

Update on "Sparse Compressed Transpose add support for Batch dims and…

12bc8f4

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

35e8010

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

d9690aa

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

e93b082

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

460f8aa

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

7134767

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

79b0fd9

… BSR/BSC layouts" [ghstack-poisoned]

nikitaved reviewed Aug 29, 2022

View reviewed changes

aten/src/ATen/native/TensorShape.cpp Show resolved Hide resolved

amjames added 3 commits August 30, 2022 12:19

Update on "Sparse Compressed Transpose add support for Batch dims and…

76b9602

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

d724a37

… BSR/BSC layouts" [ghstack-poisoned]

Update on "Sparse Compressed Transpose add support for Batch dims and…

906e60b

… BSR/BSC layouts" [ghstack-poisoned]

pytorchmergebot pushed a commit that referenced this pull request Sep 1, 2022

Sparse Compressed Transpose add support for Batch dims and BSR/BSC la…

6557bd8

…youts (#82122) Pull Request resolved: #82122 Approved by: https://github.com/bhosmer

pytorchmergebot pushed a commit that referenced this pull request Sep 2, 2022

Sparse Compressed Transpose add support for Batch dims and BSR/BSC la…

83207b4

…youts (#82122) Pull Request resolved: #82122 Approved by: https://github.com/bhosmer

pytorchmergebot added the Merged label Sep 2, 2022

pytorchmergebot closed this in 9b115c7 Sep 2, 2022

facebook-github-bot deleted the gh/amjames/11/head branch September 6, 2022 14:20

SherlockNoMad mentioned this pull request Sep 8, 2022

Return x.alias() when transpose is an nop #84674

Closed

cpuhrsch reviewed Sep 8, 2022

View reviewed changes

amjames commented Sep 9, 2022

View reviewed changes

amjames mentioned this pull request Sep 16, 2022

Transposing a sparse CSR tensor changes the CUDA index of the tensor #84999

Closed

nikitaved mentioned this pull request Dec 5, 2022

Implement hybrid sparse to/from dense conversions. #90177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts #82122

Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts #82122

amjames commented Jul 25, 2022 •

edited

Loading

facebook-github-bot commented Jul 25, 2022 •

edited

Loading

🕵️‍♀️ 1 failure not recognized by patterns:

amjames Jul 29, 2022

bhosmer left a comment

bhosmer Aug 3, 2022

amjames commented Sep 1, 2022

pytorchmergebot commented Sep 1, 2022

pytorchmergebot commented Sep 1, 2022

amjames commented Sep 2, 2022

pytorchmergebot commented Sep 2, 2022

github-actions bot commented Sep 2, 2022

cpuhrsch Sep 8, 2022

amjames Sep 9, 2022

cpuhrsch Sep 9, 2022

Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts #82122

Sparse Compressed Transpose add support for Batch dims and BSR/BSC layouts #82122

Conversation

amjames commented Jul 25, 2022 • edited Loading

facebook-github-bot commented Jul 25, 2022 • edited Loading

🔗 Helpful links

❌ 1 New Failures

🕵️‍♀️ 1 failure not recognized by patterns:

amjames Jul 29, 2022

Choose a reason for hiding this comment

bhosmer left a comment

Choose a reason for hiding this comment

bhosmer Aug 3, 2022

Choose a reason for hiding this comment

amjames commented Sep 1, 2022

pytorchmergebot commented Sep 1, 2022

pytorchmergebot commented Sep 1, 2022

Merge failed

amjames commented Sep 2, 2022

pytorchmergebot commented Sep 2, 2022

github-actions bot commented Sep 2, 2022

cpuhrsch Sep 8, 2022

Choose a reason for hiding this comment

amjames Sep 9, 2022

Choose a reason for hiding this comment

cpuhrsch Sep 9, 2022

Choose a reason for hiding this comment

amjames commented Jul 25, 2022 •

edited

Loading

facebook-github-bot commented Jul 25, 2022 •

edited

Loading