Added CUDA support for complex input for torch.solve #47045

IvanYashchuk · 2020-10-29T10:12:25Z

torch.solve now works for complex inputs on GPU.
I moved the existing tests to test_linalg.py and modified them to test complex and float32 dtypes.
Differentiation also works correctly with complex inputs.

Fixes #41084
Ref. #33152

@anjali411 I hope you don't mind that I took over #42737

anjali411 · 2020-10-29T15:03:11Z

test/test_linalg.py

+    @skipCPUIfNoLapack
+    @dtypes(torch.float64, torch.complex128)
+    @dtypesIfCUDA(torch.float64)
+    def test_solve_autograd(self, device, dtype):


I don't think we should add autograd tests in test_linalg.py. The only autograd tests this file are in test_autograd_and_jit which are tagged with a note that they should be moved to method_tests in common_methods_invocations.py cc. @mruberry

for now we should just add tests in method_tests for solve by adding an entry here: https://github.com/pytorch/pytorch/blob/master/test/test_autograd.py#L4928-L4929 and adding complex specific tests similar to this (make sure to include 'complex' in the substring for each of the new tests so that they only run for complex dtype).

It might also be worth adding LinalgFuncInfo to migrate the linalg autograd tests as we are extending the linalg support but it's ok to leave that for a follow-up PR.

I recall @mruberry mentioned in other PRs to move all linalg related tests to test_linalg.py, so I think it includes also autograd tests that are not part of method_tests, and not only from test_torch.py.
Since in the future, the plan is to have LinalgFuncInfo instead of method_tests, I suggest we leave this test here and have CPU&CUDA variant tested without adding an entry to method_tests (which is a bit troublesome because we need to create an input with the dtype and device using random_fullrank_matrix_distinct_singular_value(..., dtype, device).

We often debate amongst ourselves where tests related to autograd should live. Do they live with the operator, or do they live with test_autograd.py? And we haven't come to a consistent resolution yet, unfortunately, so it's reviewers' discretion. In the (near) future I expect we'll publicize a definitive plan for where these tests go. Sorry there's not a clear solution at the moment.

anjali411 · 2020-10-29T15:04:32Z

test/test_torch.py

@@ -7614,87 +7614,6 @@ def run_test(matsize, batchdims, mat_chars):
            run_test(matsize, batchdims, mat_chars=['sym', 'sym_pd', 'sym_psd'])
            run_test(matsize, batchdims, mat_chars=['sing', 'non_sing'])

-    def solve_test_helper(self, A_dims, b_dims, device, dtype):


thanks! good to see tests being migrated from test_torch.py to specific dedicated test files like test_linalg.py.

anjali411

lgtm once the autograd tests are removed from test_linalg.py and added to method_tests.

facebook-github-bot · 2020-10-30T17:30:28Z

Hi @IvanYashchuk!

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but we do not have a signature on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

codecov · 2020-11-02T09:42:02Z

Codecov Report

Merging #47045 (2d37d7f) into master (2df5600) will increase coverage by 39.16%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master   #47045       +/-   ##
===========================================
+ Coverage   42.06%   81.23%   +39.16%     
===========================================
  Files         472     1837     +1365     
  Lines       64759   198089   +133330     
===========================================
+ Hits        27240   160909   +133669     
+ Misses      37519    37180      -339

dr-ci · 2020-11-02T09:49:15Z

💊 CI failures summary and remediations

As of commit 2d37d7f (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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 28 times.

IvanYashchuk · 2020-11-02T09:49:43Z

@anjali411 I've removed the autograd tests from test_linalg.py and added to method_tests.

IvanYashchuk · 2020-11-02T11:50:03Z

test/test_linalg.py

    def test_tensorsolve_empty(self, device, dtype):
        # Check for empty inputs. NumPy does not work for these cases.
        a = torch.empty(0, 0, 1, 2, 3, 0, dtype=dtype, device=device)
        b = torch.empty(a.shape[:2], dtype=dtype, device=device)
        x = torch.linalg.tensorsolve(a, b)
        self.assertEqual(torch.tensordot(a, x, dims=len(x.shape)), b)

-    # TODO: once "solve_cuda" supports complex dtypes, they shall be added to above tests


torch.linalg.tensorsolve now works for complex dtypes.

mruberry · 2020-11-06T04:41:03Z

Ping @anjali411 but @IvanYashchuk this needs a rebase.

IvanYashchuk · 2020-11-06T07:20:25Z

@mruberry, we've decided with Anjali that we'll merge these complex-valued CUDA PRs in turn in this order:

anjali411

lgtm (but just needs a rebase) thank you @IvanYashchuk!

Removed unused imports

facebook-github-bot

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

facebook-github-bot

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

facebook-github-bot · 2020-11-12T21:18:54Z

@anjali411 merged this pull request in 149190c.

Summary: `torch.solve` now works for complex inputs on GPU. I moved the existing tests to `test_linalg.py` and modified them to test complex and float32 dtypes. Differentiation also works correctly with complex inputs. Fixes pytorch#41084 Ref. pytorch#33152 anjali411 I hope you don't mind that I took over pytorch#42737 Pull Request resolved: pytorch#47045 Reviewed By: nikithamalgifb Differential Revision: D24921503 Pulled By: anjali411 fbshipit-source-id: 4c3fc4f193a84b6e28c43c08672d480715000923

IvanYashchuk added 5 commits October 29, 2020 04:09

Added complex solve for cuda

14723e1

Add supports note to docs

bd40a65

Moved tests to test_linalg.py

cd9dc44

Added complex and fp32 dtype test cases

0c50e55

Update derivative rule for solve, added autograd tests

d417e34

IvanYashchuk added module: complex Related to complex number support in PyTorch module: linear algebra Issues related to specialized linear algebra operations in PyTorch; includes matrix multiply matmul complex_autograd labels Oct 29, 2020

IvanYashchuk requested a review from anjali411 October 29, 2020 10:12

IvanYashchuk requested review from albanD and apaszke as code owners October 29, 2020 10:12

IvanYashchuk removed request for apaszke and albanD October 29, 2020 10:12

pytorchbot added the open source label Oct 29, 2020

albanD added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 29, 2020

anjali411 reviewed Oct 29, 2020

View reviewed changes

anjali411 approved these changes Oct 29, 2020

View reviewed changes

facebook-github-bot added the cla signed label Oct 31, 2020

IvanYashchuk added 3 commits November 2, 2020 02:49

Merge remote-tracking branch 'upstream/master' into complex-solve

c6df213

Removed autograd tests from test_linalg.py

7eb45d6

Added complex tests via common_methods_invocations.py

15950ff

flake8

cd3bd74

IvanYashchuk requested a review from anjali411 November 2, 2020 09:49

IvanYashchuk added 2 commits November 2, 2020 03:58

Remove unused import

70b6625

Fix xfailing test cases for tensorsolve

b0c6388

Fix supports note in tensorsolve docs

7d8f6e7

IvanYashchuk commented Nov 2, 2020

View reviewed changes

IvanYashchuk and others added 2 commits November 2, 2020 06:16

Remove unused import

f260b98

Merge branch 'master' into complex-solve

682a954

Merge remote-tracking branch 'upstream/master' into complex-solve

c626715

anjali411 approved these changes Nov 12, 2020

View reviewed changes

IvanYashchuk added 2 commits November 12, 2020 11:07

Merge branch 'master' into complex-solve

917a058

Update test_linalg.py

924ff57

Removed unused imports

facebook-github-bot reviewed Nov 12, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into complex-solve

2d37d7f

facebook-github-bot reviewed Nov 12, 2020

View reviewed changes

facebook-github-bot closed this in 149190c Nov 12, 2020

facebook-github-bot added the Merged label Nov 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added CUDA support for complex input for torch.solve #47045

Added CUDA support for complex input for torch.solve #47045

IvanYashchuk commented Oct 29, 2020

anjali411 Oct 29, 2020

anjali411 Oct 29, 2020

anjali411 Oct 29, 2020

IvanYashchuk Oct 30, 2020

mruberry Oct 30, 2020

anjali411 Oct 29, 2020

anjali411 left a comment •

edited

facebook-github-bot commented Oct 30, 2020

codecov bot commented Nov 2, 2020 •

edited

dr-ci bot commented Nov 2, 2020 •

edited

IvanYashchuk commented Nov 2, 2020

IvanYashchuk Nov 2, 2020

mruberry commented Nov 6, 2020

IvanYashchuk commented Nov 6, 2020 •

edited by anjali411

anjali411 left a comment

facebook-github-bot left a comment

facebook-github-bot left a comment

facebook-github-bot commented Nov 12, 2020

Added CUDA support for complex input for torch.solve #47045

Added CUDA support for complex input for torch.solve #47045

Conversation

IvanYashchuk commented Oct 29, 2020

anjali411 Oct 29, 2020

Choose a reason for hiding this comment

anjali411 Oct 29, 2020

Choose a reason for hiding this comment

anjali411 Oct 29, 2020

Choose a reason for hiding this comment

IvanYashchuk Oct 30, 2020

Choose a reason for hiding this comment

mruberry Oct 30, 2020

Choose a reason for hiding this comment

anjali411 Oct 29, 2020

Choose a reason for hiding this comment

anjali411 left a comment • edited

Choose a reason for hiding this comment

facebook-github-bot commented Oct 30, 2020

codecov bot commented Nov 2, 2020 • edited

Codecov Report

dr-ci bot commented Nov 2, 2020 • edited

💊 CI failures summary and remediations

IvanYashchuk commented Nov 2, 2020

IvanYashchuk Nov 2, 2020

Choose a reason for hiding this comment

mruberry commented Nov 6, 2020

IvanYashchuk commented Nov 6, 2020 • edited by anjali411

anjali411 left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Nov 12, 2020

anjali411 left a comment •

edited

codecov bot commented Nov 2, 2020 •

edited

dr-ci bot commented Nov 2, 2020 •

edited

IvanYashchuk commented Nov 6, 2020 •

edited by anjali411