[Ready] Make potrs batched #13453

vishwakftw · 2018-11-01T16:49:43Z

This is a straightforward PR, building up on the batch inverse PR, except for one change:
- The GENERATE_LINALG_HELPER_n_ARGS macro has been removed, since it is not very general and the resulting code is actually not very copy-pasty.

Billing of changes:

Add batching for potrs
Add relevant tests
Modify doc string

Minor changes:

Remove _gesv_single, _getri_single from aten_interned_strings.h.
Add test for CUDA potrs (2D Tensor op)
Move the batched shape checking to LinearAlgebraUtils.h

test/test_cuda.py

- This is straightforward PR, building up on the batch inverse PR, except for one change: - The GENERATE_LINALG_HELPER_n_ARGS macro has been removed, since it is not very general and the resulting code is actually not very copy-pasty.

vishwakftw · 2018-11-03T17:36:24Z

@zou3519 This is ready for review, just for your information.

zou3519 · 2018-11-05T15:55:06Z

@vishwakftw I'll take a look later today or tomorrow

vishwakftw · 2018-11-06T06:35:19Z

Thank you, appreciate it. :-)

zou3519 · 2018-11-06T16:29:31Z

No, thank you for your contribution :)

aten/src/ATen/Declarations.cwrap

aten/src/ATen/native/BatchLinearAlgebra.cpp


 template<class scalar_t>
-void lapackGetri(int n, scalar_t *a, int lda, int *ipiv, scalar_t *work, int lwork, int *info) {
+void lapackGetri(int n, scalar_t* a, int lda, int* ipiv, scalar_t* work, int lwork, int* info) {


facebook-github-bot

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

aten/src/ATen/native/cuda/BatchLinearAlgebra.cu

tools/autograd/gen_python_functions.py

    '_cumsum.*', '_cumprod.*', '_sum.*', '_prod.*', '_th_.*',
-    'arange.*', 'range.*', '_gesv.*', '_getri.*', '_inverse.*', 'slice',
-    'randint(_out)?',
+    'arange.*', 'range.*', '_gesv.*', '_getri.*', '_inverse.*', '_potrs.*',


torch/_torch_docs.py

tools/autograd/derivatives.yaml

test/common_utils.py

test/test_torch.py

+            b = cast(torch.randn(2, 1, 3, 4, 6))
+            L = get_cholesky(A, upper)
+            x = torch.potrs(b, L, upper=upper)
+            x_exp = torch.Tensor(solve(A.cpu().numpy(), b.cpu().numpy()))


test/test_torch.py

+            self.assertEqual(x.data, cast(x_exp))
+
+            # broadcasting A
+            A = cast(random_symmetric_pd_matrix(4))


test/test_torch.py

- minor refactor for potrs tests

zou3519

lgtm, thank you @vishwakftw!

facebook-github-bot

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

vishwakftw · 2018-11-09T13:18:46Z

@zou3519 is there anything that I need to do?

facebook-github-bot

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

zou3519 · 2018-11-09T14:56:29Z

@vishwakftw I think it should be good, I'll let you know if any action is required

vishwakftw · 2018-11-09T16:01:16Z

@zou3519 just a notification: there were merge conflicts after the recent changes to Tensor.h/TensorMethods.h/Type.h. I fixed them.

facebook-github-bot

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

Summary: - This is a straightforward PR, building up on the batch inverse PR, except for one change: - The GENERATE_LINALG_HELPER_n_ARGS macro has been removed, since it is not very general and the resulting code is actually not very copy-pasty. Billing of changes: - Add batching for `potrs` - Add relevant tests - Modify doc string Minor changes: - Remove `_gesv_single`, `_getri_single` from `aten_interned_strings.h`. - Add test for CUDA `potrs` (2D Tensor op) - Move the batched shape checking to `LinearAlgebraUtils.h` Pull Request resolved: pytorch/pytorch#13453 Reviewed By: soumith Differential Revision: D12942039 Pulled By: zou3519 fbshipit-source-id: 1b8007f00218e61593fc415865b51c1dac0b6a35

vishwakftw force-pushed the batch_potrs branch from 00d213d to b4ff163 Compare November 2, 2018 06:57

vishwakftw commented Nov 2, 2018

View reviewed changes

test/test_cuda.py Outdated

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

vishwakftw force-pushed the batch_potrs branch from b4ff163 to 314f875 Compare November 2, 2018 07:08

vishwakftw added 3 commits November 3, 2018 19:03

Make potrs batched

f9f6d69

- This is straightforward PR, building up on the batch inverse PR, except for one change: - The GENERATE_LINALG_HELPER_n_ARGS macro has been removed, since it is not very general and the resulting code is actually not very copy-pasty.

Add tests for batched potrs and modify doc strings for batched behaviour

3fe5df3

Complete tests for potrs

551360a

vishwakftw force-pushed the batch_potrs branch from 314f875 to 551360a Compare November 3, 2018 15:16

vishwakftw changed the title ~~[WIP] Make potrs batched~~ [Ready] Make potrs batched Nov 3, 2018

Fix lint

b8914dc

zou3519 self-assigned this Nov 5, 2018

zou3519 self-requested a review November 6, 2018 16:29

zou3519 removed their assignment Nov 6, 2018