Added linalg.slogdet #49194

IvanYashchuk · 2020-12-10T22:31:28Z

This PR adds torch.linalg.slogdet.

Changes compared to the original torch.slogdet:

Complex input now works as in NumPy
Added out= variant (allocates temporary and makes a copy for now)
Updated slogdet_backward to work with complex input

Ref. #42666

facebook-github-bot · 2020-12-10T22:31:56Z

💊 CI failures summary and remediations

As of commit 8fbdf60 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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

Follow this link to opt-out of these comments for your Pull Requests.

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

This comment has been revised 93 times.

mruberry · 2020-12-15T09:03:36Z

This test failure looks real:

test_slogdet_cpu_float32 - TestLinalgCPU
test_linalg.py

Traceback (most recent call last):
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 273, in instantiated_test
    result = test_fn(self, *args)
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 545, in dep_fn
    return fn(slf, device, *args, **kwargs)
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_device_type.py", line 545, in dep_fn
    return fn(slf, device, *args, **kwargs)
  File "test_linalg.py", line 4726, in test_slogdet
    run_test(matsize, batchdims, mat_chars=['singular'])
  File "test_linalg.py", line 4712, in run_test
    self.assertEqual(expected_value[0], actual_value[0], atol=self.precision, rtol=self.precision)
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 1187, in assertEqual
    exact_dtype=exact_dtype, exact_device=exact_device)
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 1068, in assertEqual
    exact_dtype=exact_dtype, exact_device=exact_device)
  File "/opt/conda/lib/python3.6/site-packages/torch/testing/_internal/common_utils.py", line 1198, in assertEqual
    super().assertTrue(result, msg=self._get_assert_msg(msg, debug_msg=debug_msg))
AssertionError: False is not true : Scalars failed to compare as equal! Comparing 1.0 and 0.0 gives a difference of 1.0, but the allowed difference with rtol=0.001 and atol=0.001 is only 0.001!

mruberry · 2020-12-15T09:04:42Z

aten/src/ATen/native/LinearAlgebra.cpp

  squareCheckInputs(self);
-  TORCH_CHECK((at::isFloatingType(self.scalar_type()) || at::isComplexType(self.scalar_type())),
-              "Expected a floating point tensor as input");
+  TORCH_CHECK((at::isFloatingType(self.scalar_type()) || at::isComplexType(self.scalar_type())) && self.dim() >= 2,


Does this handle bfloat16 and float16 inputs, too?

No, this was copied from the old code of slogdet and I didn't check what isFloatingType does.
I'll change it to accept only float, double, cfloat, cdouble.

aten/src/ATen/native/LinearAlgebra.cpp

codecov · 2021-01-07T16:46:12Z

Codecov Report

Merging #49194 (5440f7a) into master (30e45bb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #49194   +/-   ##
=======================================
  Coverage   80.67%   80.68%           
=======================================
  Files        1910     1910           
  Lines      207864   207890   +26     
=======================================
+ Hits       167697   167727   +30     
+ Misses      40167    40163    -4

IvanYashchuk · 2021-01-07T18:38:59Z

@mruberry, holidays are over and I've finally updated this pull request. It's ready for another round of review.

mruberry · 2021-01-13T09:55:52Z

Looks like this just needs a rebase. Also, I thought we had to register complex gradients somewhere? Did I miss that or is it not needed in this PR?

Sorry for the delay in reviewing this, by the way. It's been an incredibly busy week. Things should get back to normal next week.

mruberry

Awesome.

Sorry for the delay in reviewing this, @IvanYashchuk. It's been an incredibly busy week, and I appreciate your help reviewing cuSOLVER svd and lstsq. Things should be back to normal next week, but if you're enjoying helping with reviews it'd be great to have you review more often. Let me know.

On this PR, I thought supporting complex autograd required adding the function's name to a list somewhere - did I miss that in this PR or does linalg.slogdet not need to do that for some reason?

Finally, this just needs a rebase (edit: and let's remove some method_test entries).

mruberry · 2021-01-13T10:09:07Z

torch/testing/_internal/common_methods_invocations.py

+           skips=(
+               # These tests do not work with output_func=itemgetter(1)
+               # TODO: remove this once https://github.com/pytorch/pytorch/issues/49326 is resolved
+               SkipInfo('TestCommon', 'test_variant_consistency_jit'),)),


While we're here let's remove the slogdet entries in method_tests:

pytorch/torch/testing/_internal/common_methods_invocations.py

Line 2295 in 4a3a378

('slogdet', lambda dtype, device: make_nonzero_det(torch.randn(1, 1), 1), NO_ARGS,

slogdet is just a deprecated alias for linalg.slogdet at this point.

IvanYashchuk · 2021-01-14T22:25:11Z

@mruberry I removed method_test entries.
linalg.slogdet does not need to be registered for complex autograd because this function maps complex-valued input to real-valued output and it's not differentiable wrt sign (which is complex). See https://github.com/pytorch/pytorch/blob/master/tools/autograd/gen_variable_type.py#L69-L70

if you're enjoying helping with reviews it'd be great to have you review more often. Let me know.

Sure! Assign me as a reviewer wherever you think I can help

facebook-github-bot

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

mruberry · 2021-01-18T07:43:18Z

Import is needed:

Jan 15 01:13:12   File "test_ops.py", line 60, in <module>
Jan 15 01:13:12     _gradcheck_ops = partial(ops, dtypes=OpDTypes.supported,
Jan 15 01:13:12 NameError: name 'partial' is not defined

IvanYashchuk · 2021-01-18T08:05:16Z

@mruberry I am sorry I overlooked that.

mruberry · 2021-01-19T09:31:52Z

@mruberry I am sorry I overlooked that.

No worries. Happens to everyone.

facebook-github-bot

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

facebook-github-bot · 2021-01-19T15:30:11Z

@mruberry merged this pull request in f9a5ba7.

Summary: Fixes #51652. In particular: - the main implementation is in `torch.linalg.det` now. `torch.det` is just a deprecated alias to it - add a new `OpInfo` for `torch.linalg.det` - remove the old-style tests for `torch.det` (this is similar to what we did for `torch.linalg.slogdet`, see #49194) - added a `out=` argument to `torch.linalg.det`, but **not** to `torch.det`. It is worth noting that I had to skip few tests: - `TestGradientsCuda::test_fn_gradgrad_linalg_det_cuda_float64`. This is not a regression: the functionality is broken also on master, but the test is not executed properly due to #53361. And the following tests which fails only on ROCm: - `test_variant_consistency_jit_cuda_{float64,float32}` - `test_fn_grad_cuda_float64` I think that the ROCm tests fail because the current linalg.det backward is unstable if the matrix has repeated singular values, see #53364 . (At the moment of writing some CI jobs are still running but I believe the build will be green, since the only difference wrt the last push is the skip of the ROCm tests) Pull Request resolved: #53119 Reviewed By: H-Huang Differential Revision: D27441999 Pulled By: mruberry fbshipit-source-id: 5eab14c4f0a165e0cf9ec626c3f4bb23359f2a9e

IvanYashchuk added 8 commits November 23, 2020 12:40

wip linalg.slogdet

1c21ef2

Merge remote-tracking branch 'upstream/master' into linalg-slogdet

b36e96d

Removed duplicated tests

6908118

Added linalg.slogdet to tests

53ef94b

Implemented complex input support; added tests

d9badc6

Updated docs

0a54b94

Added out= tests

81463f0

wip started OpInfo-based test; doesn't work yet

f8b4fa7

IvanYashchuk added module: numpy Related to numpy support, and also numpy compatibility of our operators module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul labels Dec 10, 2020

facebook-github-bot added the cla signed label Dec 10, 2020

IvanYashchuk mentioned this pull request Dec 10, 2020

torch.linalg in PyTorch 1.10 tracker #42666

Closed

4 tasks

pytorchbot added the open source label Dec 10, 2020

IvanYashchuk added 7 commits December 14, 2020 10:57

Updated slogdet_backward to work for complex input

ca9d505

Fix test_unsupported_dtypes

504d009

Skip test_variant_consistency_jit

0d172ca

Merge remote-tracking branch 'upstream/master' into linalg-slogdet

b941c48

flake8

e99b08b

Updated comment in tests

75d4597

Added entry to test_namedtuple_return_api.py

afc4edd

IvanYashchuk marked this pull request as ready for review December 14, 2020 11:53

IvanYashchuk requested review from albanD and glaringlee as code owners December 14, 2020 11:53

IvanYashchuk requested review from mruberry and removed request for albanD and glaringlee December 14, 2020 11:53

mruberry reviewed Dec 15, 2020

View reviewed changes

aten/src/ATen/native/LinearAlgebra.cpp Outdated Show resolved Hide resolved

IvanYashchuk added 5 commits January 7, 2021 10:30

self.dim >= 2 is alerady checked in squareCheckInputs

741146d

Make test_ops faster by changing sample_inputs_slogdet

b3f8932

partial is absorbed into 'fn'

5af14a4

Merge remote-tracking branch 'upstream/master' into linalg-slogdet

fa3b685

Removed unused imports

0ef6fb5

Merge branch 'master' into linalg-slogdet

8fbdf60

mruberry self-requested a review January 13, 2021 09:55

mruberry approved these changes Jan 13, 2021

View reviewed changes

mruberry reviewed Jan 13, 2021

View reviewed changes

IvanYashchuk added 2 commits January 14, 2021 16:15

Merge remote-tracking branch 'upstream/master' into linalg-slogdet

6d42706

Removed the slogdet entries in method_tests

b490239

facebook-github-bot reviewed Jan 14, 2021

View reviewed changes

Get partial back

5440f7a

facebook-github-bot reviewed Jan 19, 2021

View reviewed changes

facebook-github-bot closed this in f9a5ba7 Jan 19, 2021

facebook-github-bot added the Merged label Jan 19, 2021

antocuni mentioned this pull request Mar 5, 2021

torch.linalg.det #53119

Closed

IvanYashchuk mentioned this pull request Apr 20, 2021

Improved docs for torch.linalg #56265

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added linalg.slogdet #49194

Added linalg.slogdet #49194

IvanYashchuk commented Dec 10, 2020 •

edited

Loading

facebook-github-bot commented Dec 10, 2020 •

edited

Loading

mruberry commented Dec 15, 2020

mruberry Dec 15, 2020

IvanYashchuk Dec 15, 2020

codecov bot commented Jan 7, 2021 •

edited

Loading

IvanYashchuk commented Jan 7, 2021

mruberry commented Jan 13, 2021

mruberry left a comment •

edited

Loading

mruberry Jan 13, 2021

IvanYashchuk commented Jan 14, 2021 •

edited

Loading

facebook-github-bot left a comment

mruberry commented Jan 18, 2021

IvanYashchuk commented Jan 18, 2021

mruberry commented Jan 19, 2021

facebook-github-bot left a comment

facebook-github-bot commented Jan 19, 2021

Added linalg.slogdet #49194

Added linalg.slogdet #49194

Conversation

IvanYashchuk commented Dec 10, 2020 • edited Loading

facebook-github-bot commented Dec 10, 2020 • edited Loading

💊 CI failures summary and remediations

mruberry commented Dec 15, 2020

mruberry Dec 15, 2020

Choose a reason for hiding this comment

IvanYashchuk Dec 15, 2020

Choose a reason for hiding this comment

codecov bot commented Jan 7, 2021 • edited Loading

Codecov Report

IvanYashchuk commented Jan 7, 2021

mruberry commented Jan 13, 2021

mruberry left a comment • edited Loading

Choose a reason for hiding this comment

mruberry Jan 13, 2021

Choose a reason for hiding this comment

IvanYashchuk commented Jan 14, 2021 • edited Loading

facebook-github-bot left a comment

Choose a reason for hiding this comment

mruberry commented Jan 18, 2021

IvanYashchuk commented Jan 18, 2021

mruberry commented Jan 19, 2021

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jan 19, 2021

IvanYashchuk commented Dec 10, 2020 •

edited

Loading

facebook-github-bot commented Dec 10, 2020 •

edited

Loading

codecov bot commented Jan 7, 2021 •

edited

Loading

mruberry left a comment •

edited

Loading

IvanYashchuk commented Jan 14, 2021 •

edited

Loading