Allowing batching for det/logdet/slogdet operations #22909

vishwakftw · 2019-07-16T14:19:04Z

Changelog:

Add batching for det / logdet / slogdet operations
Update derivative computation to support batched inputs (and consequently batched outputs)
Update docs

Test Plan:

Add a test_det_logdet_slogdet_batched method in test_torch.py to test torch.det, torch.logdet and torch.slogdet on batched inputs. This relies on the correctness of torch.det on single matrices (tested by test_det_logdet_slogdet). A port of this test is added to test_cuda.py
Add autograd tests for batched inputs

…h-det-logdet-slogdet

vishwakftw · 2019-07-16T14:19:26Z

cc: @ssnl

calincru · 2019-07-18T19:31:33Z

Really looking forward to this, I need it as soon as possible.

ssnl · 2019-07-19T19:05:33Z

tools/autograd/templates/Functions.cpp

+  } else if ((det == 0).all().item<uint8_t>()) {  // all matrices are singular
+    return singular_case_backward(grad, self, det);
+  } else {  // some matrices are invertible, some matrices are singular
+    return at::where(det != 0,


I'm still not a fan of this. This means that just running det in a loop and cat afterwards can often be much faster.

You are synching by accessing det.all() anyways. So, why not just do a det.nonzero(), selecting and calculating both parts, and then putting them together?

I actually didn’t think of nonzero, I’ll try it out.

yeah, thanks! Inplace index_put gradients into correct locations may break double backward, you might also want to check if that still works.

Inplace index_put has a gradient defined, so I don't think it'll break double backward. I've made changes as per your suggestions.

ssnl · 2019-07-19T19:06:25Z

tools/autograd/templates/Functions.cpp

+
+  if ((det != 0).all().item<uint8_t>()) {  // all matrices are invertible
+    return unsqueeze_multiple(grad * det, {-1, -2}, self.dim()) * self.inverse().transpose(-2, -1);
+  } else if ((det == 0).all().item<uint8_t>()) {  // all matrices are singular


I'm pretty sure these two if conds can be merged together to avoid some duplicated computation.

ssnl · 2019-07-19T19:08:05Z

This is great, but in the end this is a wrapper (since there are no actual batched kernels) so users don't need to write a for loop themselves. So we shouldn't make certain cases slower than a for loop.

Gsunshine · 2019-07-20T04:14:23Z

This is great, but in the end this is a wrapper (since there are no actual batched kernels) so users don't need to write a for loop themselves. So we shouldn't make certain cases slower than a for loop.

emmm, you mean we cannot actually get acceleration from paralell computing...?

ssnl · 2019-07-20T04:21:44Z

@Gsunshine The forward is batched with magma's batched LU call, but the backward is just a loop, unless @vishwakftw is launching svd on different streams, which I don't think is happening with the current code.

vishwakftw · 2019-07-20T04:35:26Z

@Gsunshine yes, @ssnl is right about the forward call. For backward, in this case of invertible matrices, the backward uses a batch MAGMA getri cal, but in the case of singular matrices, the backward computation is looped.

Gsunshine · 2019-07-20T07:15:56Z

@ssnl @vishwakftw I got it. thx!

I prefer logdet for invertible matrices with both batched forward and backward call, while keeping a general function slogdet with svd backward. Maybe an argument sym_pos_def for SPD matrices could be added to det & logdet to enable even faster batched cholesky implementation.

If you need any help in the enhancement, pls feel free to @Gsunshine . Hope to offer my help!

…h-det-logdet-slogdet

vishwakftw · 2019-07-22T13:45:27Z

@pytorchbot rebase this please

…rch into batch-det-logdet-slogdet

…_diag_U_info - _lu_det_P_diag_U_info is renamed to _lu_det_P_diag_U

…h-det-logdet-slogdet

vishwakftw · 2019-07-29T16:20:35Z

@pytorchbot rebase this please

vishwakftw · 2019-07-29T18:49:10Z

@ssnl I optimized the forward implementation now (turns out we actually don’t need the infos tensor from the helper function to be returned).

ssnl

I have a bunch of perf comments, but they are all minor. Feel free to just merge this please after you fix them!

Also, this looks great! Thank you so much for working on this! :)

aten/src/ATen/native/LinearAlgebra.cpp

tools/autograd/templates/Functions.cpp

ssnl · 2019-07-31T01:22:05Z

These things are small so I pushed to your branch. Hopefully we can get a green CI.

vishwakftw · 2019-07-31T03:01:48Z

@ssnl thank you very much for the detailed review. Sorry I couldn’t get to them earlier than you did because it was late back here.

vishwakftw · 2019-07-31T05:49:44Z

@pytorchbot merge this please

facebook-github-bot

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Changelog: - Add batching for det / logdet / slogdet operations - Update derivative computation to support batched inputs (and consequently batched outputs) - Update docs Pull Request resolved: pytorch#22909 Test Plan: - Add a `test_det_logdet_slogdet_batched` method in `test_torch.py` to test `torch.det`, `torch.logdet` and `torch.slogdet` on batched inputs. This relies on the correctness of `torch.det` on single matrices (tested by `test_det_logdet_slogdet`). A port of this test is added to `test_cuda.py` - Add autograd tests for batched inputs Differential Revision: D16580988 Pulled By: ezyang fbshipit-source-id: b76c87212fbe621f42a847e3b809b5e60cfcdb7a

Summary: Changelog: - Add batching for det / logdet / slogdet operations - Update derivative computation to support batched inputs (and consequently batched outputs) - Update docs Pull Request resolved: pytorch/pytorch#22909 Test Plan: - Add a `test_det_logdet_slogdet_batched` method in `test_torch.py` to test `torch.det`, `torch.logdet` and `torch.slogdet` on batched inputs. This relies on the correctness of `torch.det` on single matrices (tested by `test_det_logdet_slogdet`). A port of this test is added to `test_cuda.py` - Add autograd tests for batched inputs Differential Revision: D16580988 Pulled By: ezyang fbshipit-source-id: b76c87212fbe621f42a847e3b809b5e60cfcdb7a

facebook-github-bot · 2019-07-31T23:40:16Z

@ezyang merged this pull request in 5d130e4.

Summary: Changelog: - Add batching for det / logdet / slogdet operations - Update derivative computation to support batched inputs (and consequently batched outputs) - Update docs Pull Request resolved: #22909 Test Plan: - Add a `test_det_logdet_slogdet_batched` method in `test_torch.py` to test `torch.det`, `torch.logdet` and `torch.slogdet` on batched inputs. This relies on the correctness of `torch.det` on single matrices (tested by `test_det_logdet_slogdet`). A port of this test is added to `test_cuda.py` - Add autograd tests for batched inputs Differential Revision: D16580988 Pulled By: ezyang fbshipit-source-id: b76c87212fbe621f42a847e3b809b5e60cfcdb7a

vishwakftw added 4 commits July 5, 2019 13:23

Add batched det, logdet, slogdet

f432eaa

Update derivatives to be batch-mode friendly

fa0d890

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

234347e

…h-det-logdet-slogdet

Avoid executing both branches in at::where by default

3ea4608

pytorchbot added module: internals Related to internal abstractions in c10 and ATen module: operators labels Jul 16, 2019

gpleiss mentioned this pull request Jul 16, 2019

[Feature request] Batch linear algebra operators #7500

Closed

14 tasks

ezyang added the open source label Jul 16, 2019

vishwakftw added 2 commits July 19, 2019 20:43

Add tests in test_autograd and test_torch with a port to test_cuda

ce4e77e

Update docs

28d33ce

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: tests Issues related to tests (not the torch.testing module) labels Jul 19, 2019

vishwakftw requested a review from ssnl July 19, 2019 15:19

vishwakftw changed the title ~~[WIP] Allowing batching for det/logdet/slogdet operations~~ Allowing batching for det/logdet/slogdet operations Jul 19, 2019

ssnl reviewed Jul 19, 2019

View reviewed changes

cpuhrsch added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jul 19, 2019

vishwakftw added 4 commits July 20, 2019 19:17

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

eba81eb

…h-det-logdet-slogdet

Use where and index_put to avoid redundant computation

2a56900

Fix lint

6d35aaf

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

f8fdf40

…h-det-logdet-slogdet

vishwakftw added 4 commits July 28, 2019 23:00

Simplify ops

94440b0

Merge branch 'batch-det-logdet-slogdet' of github.com:vishwakftw/pyto…

a54bc27

…rch into batch-det-logdet-slogdet

Remove unwanted case analysis and stop returning infos from _lu_det_P…

ac715d9

…_diag_U_info - _lu_det_P_diag_U_info is renamed to _lu_det_P_diag_U

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

5f29e63

…h-det-logdet-slogdet

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

3b83c46

ssnl approved these changes Jul 30, 2019

View reviewed changes

ssnl force-pushed the batch-det-logdet-slogdet branch 3 times, most recently from 26d03b5 to 3ad956c Compare July 31, 2019 02:10

Small fixes

874c259

ssnl force-pushed the batch-det-logdet-slogdet branch from 3ad956c to 874c259 Compare July 31, 2019 02:29

.all() is only available on uint8 and bool tensors

d7f7a1c

pytorchbot added the merge-this-please Was marked for merge with @pytorchbot merge this please label Jul 31, 2019

facebook-github-bot reviewed Jul 31, 2019

View reviewed changes

facebook-github-bot closed this in 5d130e4 Jul 31, 2019

vishwakftw mentioned this pull request Jul 31, 2019

[v1.2.0] Release Tracker #23555

Closed

vishwakftw deleted the batch-det-logdet-slogdet branch July 31, 2019 17:27

facebook-github-bot added the merged label Jul 31, 2019

vishwakftw mentioned this pull request Jul 31, 2019

[v1.2.0] Allowing batching for det/logdet/slogdet operations (#22909) #23634

Merged

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing batching for det/logdet/slogdet operations #22909

Allowing batching for det/logdet/slogdet operations #22909

vishwakftw commented Jul 16, 2019 •

edited

Loading

vishwakftw commented Jul 16, 2019

calincru commented Jul 18, 2019

ssnl Jul 19, 2019

vishwakftw Jul 20, 2019

ssnl Jul 20, 2019

vishwakftw Jul 22, 2019

ssnl Jul 19, 2019

ssnl commented Jul 19, 2019 •

edited

Loading

Gsunshine commented Jul 20, 2019

ssnl commented Jul 20, 2019

vishwakftw commented Jul 20, 2019

Gsunshine commented Jul 20, 2019 •

edited

Loading

vishwakftw commented Jul 22, 2019

vishwakftw commented Jul 29, 2019

vishwakftw commented Jul 29, 2019

ssnl left a comment

ssnl commented Jul 31, 2019

vishwakftw commented Jul 31, 2019

vishwakftw commented Jul 31, 2019

facebook-github-bot left a comment

facebook-github-bot commented Jul 31, 2019

Allowing batching for det/logdet/slogdet operations #22909

Allowing batching for det/logdet/slogdet operations #22909

Conversation

vishwakftw commented Jul 16, 2019 • edited Loading

vishwakftw commented Jul 16, 2019

calincru commented Jul 18, 2019

ssnl Jul 19, 2019

Choose a reason for hiding this comment

vishwakftw Jul 20, 2019

Choose a reason for hiding this comment

ssnl Jul 20, 2019

Choose a reason for hiding this comment

vishwakftw Jul 22, 2019

Choose a reason for hiding this comment

ssnl Jul 19, 2019

Choose a reason for hiding this comment

ssnl commented Jul 19, 2019 • edited Loading

Gsunshine commented Jul 20, 2019

ssnl commented Jul 20, 2019

vishwakftw commented Jul 20, 2019

Gsunshine commented Jul 20, 2019 • edited Loading

vishwakftw commented Jul 22, 2019

vishwakftw commented Jul 29, 2019

vishwakftw commented Jul 29, 2019

ssnl left a comment

Choose a reason for hiding this comment

ssnl commented Jul 31, 2019

vishwakftw commented Jul 31, 2019

vishwakftw commented Jul 31, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 31, 2019

vishwakftw commented Jul 16, 2019 •

edited

Loading

ssnl commented Jul 19, 2019 •

edited

Loading

Gsunshine commented Jul 20, 2019 •

edited

Loading