Enable broadcasting of batch dimensions RHS and LHS tensors for lu_solve #24333

vishwakftw · 2019-08-14T15:29:42Z

Changelog:

Enable broadcasting of RHS and LHS tensors for lu_solve. This means that you can now have RHS with size 3 x 2 and LHS with size 4 x 3 x 3 for instance
Remove deprecated behavior of having 2D tensors for RHS. Now all tensors have to have a last dimension which equals the number of right hand sides
Modified docs

Test Plan:

Add tests for new behavior in test_torch.py with a port to test_cuda.py

Changelog: - Enable broadcasting of RHS and LHS tensors for lu_solve. This means that you can now have RHS with size `3 x 2` and LHS with size `4 x 3 x 3` for instance - Remove deprecated behavior of having 2D tensors for RHS. Now all tensors have to have a last dimension which equals the number of right hand sides - Modified docs Test Plan: - Add tests for new behavior in test_torch.py with a port to test_cuda.py

vishwakftw · 2019-08-15T21:29:24Z

@pytorchbot rebase this please

vishwakftw · 2019-08-18T23:15:01Z

@pytorchbot rebase this please

zou3519

Code looks correct. I had some comments on style and cleaning up the testing code

torch/_torch_docs.py

test/test_torch.py

aten/src/ATen/native/BatchLinearAlgebra.cpp

…olve-new-version

…into lu_solve-new-version

vishwakftw · 2019-08-20T17:54:28Z

@zou3519 I actually missed out on adding tests for broadcasting behavior earlier. I've added them now, just FYI.

vishwakftw · 2019-08-20T21:24:49Z

@zou3519 except for test refactoring and specifying sizes in error message (which will be addressed in follow-up PRs), the PR should be good to review again.

vishwakftw · 2019-08-20T22:47:06Z

test/common_utils.py

        return torch.stack(all_matrices).reshape(*(batches + (l, l)))


+def random_linalg_solve_processed_inputs(A_dims, b_dims, gen_fn, transform_fn, cast_fn):


Once this PR is merged, I will use this function in other places in the test suite for *solve methods. This would reduce duplication of code.

vishwakftw · 2019-08-21T23:32:51Z

@pytorchbot rebase this please

zou3519

Thank you, the code looks a lot better now :)
I had some last comments about repetitiveness in the testing code

test/test_torch.py

…into lu_solve-new-version

…olve-new-version

vishwakftw · 2019-08-29T22:07:59Z

@pytorchbot rebase this please

zou3519

Nice! Thank you

facebook-github-bot

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

…lve (#24333) Summary: Changelog: - Enable broadcasting of RHS and LHS tensors for lu_solve. This means that you can now have RHS with size `3 x 2` and LHS with size `4 x 3 x 3` for instance - Remove deprecated behavior of having 2D tensors for RHS. Now all tensors have to have a last dimension which equals the number of right hand sides - Modified docs Pull Request resolved: pytorch/pytorch#24333 Test Plan: - Add tests for new behavior in test_torch.py with a port to test_cuda.py Differential Revision: D17165463 Pulled By: zou3519 fbshipit-source-id: cda5d5496ddb29ed0182bab250b5d90f8f454aa6

facebook-github-bot · 2019-09-03T23:37:36Z

@zou3519 merged this pull request in 1e4832f.

Summary: Changelog: - De-duplicate the code in tests for torch.solve, torch.cholesky_solve, torch.triangular_solve - Skip tests explicitly if requirements aren't met for e.g., if NumPy / SciPy aren't available in the environment - Add generic helpers for these tests in test/common_utils.py Pull Request resolved: #25733 Test Plan: - All tests should pass to confirm that the change is not erroneous Clears one point specified in the discussion in #24333. Differential Revision: D17315330 Pulled By: zou3519 fbshipit-source-id: c72a793e89af7e2cdb163521816d56747fd70a0e

pytorchbot added module: cuda Related to torch.cuda, and CUDA support in general module: docs Related to our documentation, both in docs/ and docblocks module: operators labels Aug 14, 2019

vishwakftw requested a review from zou3519 August 14, 2019 18:25

Merge remote-tracking branch 'origin/master' into HEAD

0a03278

gpleiss mentioned this pull request Aug 16, 2019

[Feature request] Batch linear algebra operators #7500

Closed

14 tasks

Merge remote-tracking branch 'origin/master' into HEAD

505d36a

zou3519 reviewed Aug 19, 2019

View reviewed changes

vishwakftw added 3 commits August 20, 2019 13:39

Code Review - I

2bc9c9f

Merge branch 'master' of https://github.com/pytorch/pytorch into lu_s…

bfd1076

…olve-new-version

Merge branch 'lu_solve-new-version' of github.com:vishwakftw/pytorch …

c99dd0e

…into lu_solve-new-version

pytorchbot added the module: tests Issues related to tests (not the torch.testing module) label Aug 20, 2019

vishwakftw requested a review from zou3519 August 20, 2019 21:24

vishwakftw commented Aug 20, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into HEAD

f36b921

zou3519 reviewed Aug 22, 2019

View reviewed changes

test/test_torch.py Outdated Show resolved Hide resolved

test/test_torch.py Outdated Show resolved Hide resolved

test/test_torch.py Outdated Show resolved Hide resolved

test/test_torch.py Outdated Show resolved Hide resolved

vishwakftw added 7 commits August 22, 2019 21:09

Code review - II

bb9b072

Merge branch 'lu_solve-new-version' of github.com:vishwakftw/pytorch …

a2eaba5

…into lu_solve-new-version

Fix call to diag

4f60e65

Add lu_solve_test_helper to de-duplicate code

ae0febe

Remove line

db02de4

Fix helper reference

650b626

Merge branch 'master' of https://github.com/pytorch/pytorch into lu_s…

b4e8297

…olve-new-version

Merge remote-tracking branch 'origin/master' into HEAD

1652ea3

vishwakftw requested a review from zou3519 August 30, 2019 13:30

zou3519 approved these changes Sep 3, 2019

View reviewed changes

facebook-github-bot reviewed Sep 3, 2019

View reviewed changes

facebook-github-bot closed this in 1e4832f Sep 3, 2019

vishwakftw deleted the lu_solve-new-version branch September 3, 2019 22:18

facebook-github-bot added the merged label Sep 3, 2019

vishwakftw added the open source label Sep 5, 2019

vishwakftw mentioned this pull request Sep 5, 2019

Refactor torch.*solve tests #25733

Closed

mruberry added the Merged label Oct 28, 2020

		return torch.stack(all_matrices).reshape(*(batches + (l, l)))


		def random_linalg_solve_processed_inputs(A_dims, b_dims, gen_fn, transform_fn, cast_fn):

Enable broadcasting of batch dimensions RHS and LHS tensors for lu_solve #24333

Enable broadcasting of batch dimensions RHS and LHS tensors for lu_solve #24333

Uh oh!

Conversation

vishwakftw commented Aug 14, 2019

Uh oh!

vishwakftw commented Aug 15, 2019

Uh oh!

vishwakftw commented Aug 18, 2019

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishwakftw commented Aug 20, 2019

Uh oh!

vishwakftw commented Aug 20, 2019

Uh oh!

vishwakftw Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

vishwakftw commented Aug 21, 2019

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vishwakftw commented Aug 29, 2019

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants