Enable upper for torch.linalg.cholesky #62434

walterddr · 2021-07-29T20:35:50Z

Fixes #61988

facebook-github-bot · 2021-07-29T20:49:39Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/62434
📄 Preview docs built from this PR

💊 CI failures summary and remediations

As of commit 49f485a (more details on the Dr. CI page):

1/2 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)
1/2 tentatively recognized as flaky ❄️
- Click here to rerun these jobs

❄️ 1 failure tentatively classified as flaky

but reruns have not yet been triggered to confirm:

pytorch_linux_xenial_cuda11_1_cudnn8_py3_gcc7_test2 (1/1)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun) ❄️

Aug 06 23:07:04 unknown file: Failure

Aug 06 23:07:04 frame #9: build/bin/test_api() [0xbdb39a]
Aug 06 23:07:04 frame #10: build/bin/test_api() [0xbdbbed]
Aug 06 23:07:04 frame #11: testing::internal::UnitTestImpl::RunAllTests() + 0xe27 (0xbe47b7 in build/bin/test_api)
Aug 06 23:07:04 frame #12: testing::UnitTest::Run() + 0x8f (0xbe4b6f in build/bin/test_api)
Aug 06 23:07:04 frame #13: main + 0xc8 (0x586cb8 in build/bin/test_api)
Aug 06 23:07:04 frame #14: __libc_start_main + 0xf0 (0x7f24a3f76840 in /lib/x86_64-linux-gnu/libc.so.6)
Aug 06 23:07:04 frame #15: _start + 0x29 (0x5c7b29 in build/bin/test_api)
Aug 06 23:07:04 " thrown in the test body.
Aug 06 23:07:04 [  FAILED  ] IMethodTest.CallMethod (3 ms)
Aug 06 23:07:04 [ RUN      ] IMethodTest.GetArgumentNames
Aug 06 23:07:04 unknown file: Failure
Aug 06 23:07:04 C++ exception with description "open file failed, file path: torch/csrc/deploy/example/generated/simple_jit
Aug 06 23:07:04 Exception raised from RAIIFile at /var/lib/jenkins/workspace/caffe2/serialize/file_adapter.cc:13 (most recent call first):
Aug 06 23:07:04 frame #0: c10::Error::Error(c10::SourceLocation, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) + 0x6b (0x7f24a468c4ab in /var/lib/jenkins/workspace/build/lib/libc10.so)
Aug 06 23:07:04 frame #1: c10::detail::torchCheckFail(char const*, char const*, unsigned int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) + 0xce (0x7f24a4687c6e in /var/lib/jenkins/workspace/build/lib/libc10.so)
Aug 06 23:07:04 frame #2: caffe2::serialize::FileAdapter::RAIIFile::RAIIFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) + 0xdc (0x7f24c08c77fc in /var/lib/jenkins/workspace/build/lib/libtorch_cpu.so)
Aug 06 23:07:04 frame #3: caffe2::serialize::FileAdapter::FileAdapter(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) + 0x41 (0x7f24c08c7e81 in /var/lib/jenkins/workspace/build/lib/libtorch_cpu.so)
Aug 06 23:07:04 frame #4: torch::jit::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, c10::optional<c10::Device>, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&) + 0x40 (0x7f24c1f24b80 in /var/lib/jenkins/workspace/build/lib/libtorch_cpu.so)
Aug 06 23:07:04 frame #5: torch::jit::load(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, c10::optional<c10::Device>) + 0x6f (0x7f24c1f24d0f in /var/lib/jenkins/workspace/build/lib/libtorch_cpu.so)
Aug 06 23:07:04 frame #6: IMethodTest_GetArgumentNames_Test::TestBody() + 0x7e (0xb9f5be in build/bin/test_api)
Aug 06 23:07:04 frame #7: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 0x4a (0xbebafa in build/bin/test_api)

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm4.2-py3.6

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.

Click here to manually regenerate this comment.

lezcano

This looks good, thanks!

Could you also add some tests to make sure the argument works as we expect?

Note that we have the identity cholesky(A, upper=True) == cholesky(A).transpose(-2, -1).conj() and an equivalent one for cholesky_ex.

lezcano · 2021-07-30T13:39:37Z

torch/linalg/__init__.py

Perhaps

whether to return an upper triangular matrix. The tensor returned with upper=True is the conjugate transpose of the tensor returned with upper=False.

Same in cholesky_ex.

walterddr · 2021-07-30T21:43:13Z

aten/src/ATen/native/native_functions.yaml

these seems to be introducing backward incompatible changes to libtorch API (if i understand the CI job correctly). Should we keep those original signatures and redispatch them with default upper=False argument? did torch.cholesky have similar signature?

torch.cholesky did have a similar signature. Any change in function signature in native_functions.yaml is marked as backward-incompatible change. If we want to ignore it for now, then an entry to allow_list in test/backward_compatibility/check_backward_compatibility.py should be added.

lezcano

LGTM as well

facebook-github-bot · 2021-08-03T20:31:40Z

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

Summary: a bug was discovered in #62434, for some reason comparing the schema name didn't match the allow_list item. So: 1. remove duplicate regex compile 2. make use of the schema string is used instead of just the name Pull Request resolved: #62687 Reviewed By: ezyang Differential Revision: D30102437 Pulled By: walterddr fbshipit-source-id: 541b2ed77948f24daebb08623cadabb034a241e0

facebook-github-bot · 2021-08-06T20:19:23Z

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

facebook-github-bot · 2021-08-09T16:30:22Z

@walterddr merged this pull request in 3782f3e.

mruberry · 2021-08-11T22:09:18Z

aten/src/ATen/native/native_functions.yaml


 # "_ex" stands for experimental
- func: linalg_cholesky_ex(Tensor self, *, bool check_errors=False) -> (Tensor L, Tensor info)
+- func: linalg_cholesky_ex(Tensor self, *, bool upper=False, bool check_errors=False) -> (Tensor L, Tensor info)


This is actually an FC-breaking change but it's probably OK because this function is so new

mruberry · 2021-08-11T22:10:47Z

torch/linalg/__init__.py

+    upper (bool, optional): whether to return an upper triangular matrix.
+        The tensor returned with upper=True is the conjugate transpose of the tensor
+        returned with upper=False.
+    check_errors (bool, optional): controls whether to check the content of ``infos``. Default: `False`.


mruberry · 2021-08-11T22:11:06Z

cc @t-vi

facebook-github-bot added the cla signed label Jul 29, 2021

walterddr marked this pull request as ready for review July 29, 2021 23:19

walterddr requested review from IvanYashchuk, albanD, ezyang, lezcano, nikitaved and soulitzer as code owners July 29, 2021 23:19

ezyang removed their request for review July 30, 2021 13:23

lezcano reviewed Jul 30, 2021

View reviewed changes

walterddr force-pushed the be_add_upper_cholesky branch from 6f704f6 to 96f41fb Compare July 30, 2021 20:43

walterddr commented Jul 30, 2021

View reviewed changes

IvanYashchuk approved these changes Aug 3, 2021

View reviewed changes

lezcano approved these changes Aug 3, 2021

View reviewed changes

walterddr force-pushed the be_add_upper_cholesky branch from 96f41fb to 8852eaf Compare August 3, 2021 19:49

walterddr mentioned this pull request Aug 4, 2021

[BE] fix bc check #62687

Closed

walterddr force-pushed the be_add_upper_cholesky branch 2 times, most recently from e2729f2 to cad0eb4 Compare August 5, 2021 15:33

Rong Rong added 3 commits August 6, 2021 11:43

Enable upper for torch.linalg.cholesky

3b18eb1

address diff comments

e40782c

add skip for linalg_cholesky*

49f485a

walterddr force-pushed the be_add_upper_cholesky branch from cad0eb4 to 49f485a Compare August 6, 2021 18:43

facebook-github-bot closed this in 3782f3e Aug 9, 2021

facebook-github-bot added the Merged label Aug 9, 2021

xwang233 mentioned this pull request Aug 9, 2021

torch.linalg in PyTorch 1.10 tracker #42666

Closed

4 tasks

mruberry reviewed Aug 11, 2021

View reviewed changes

Enable upper for torch.linalg.cholesky #62434

Enable upper for torch.linalg.cholesky #62434

Uh oh!

Conversation

walterddr commented Jul 29, 2021

Uh oh!

facebook-github-bot commented Jul 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

❄️ 1 failure tentatively classified as flaky

pytorch_linux_xenial_cuda11_1_cudnn8_py3_gcc7_test2 (1/1)

ci.pytorch.org: 1 failed

Uh oh!

lezcano left a comment

Choose a reason for hiding this comment

Uh oh!

lezcano Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walterddr Jul 30, 2021

Choose a reason for hiding this comment

Uh oh!

IvanYashchuk Aug 3, 2021

Choose a reason for hiding this comment

Uh oh!

lezcano left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 3, 2021

Uh oh!

facebook-github-bot commented Aug 6, 2021

Uh oh!

facebook-github-bot commented Aug 9, 2021

Uh oh!

mruberry Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

mruberry commented Aug 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

facebook-github-bot commented Jul 29, 2021 •

edited

Loading

lezcano Jul 30, 2021 •

edited

Loading