Expand the test of torch.addbmm and torch.baddbmm by zasdfgbnm · Pull Request #47079 · pytorch/pytorch

zasdfgbnm · 2020-10-29T20:29:59Z

This is to satisfy the request at #42553 (comment). See also #47124

codecov · 2020-10-29T23:47:16Z

Codecov Report

Merging #47079 into master will increase coverage by 24.79%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master   #47079       +/-   ##
===========================================
+ Coverage   36.03%   60.82%   +24.79%     
===========================================
  Files         437     2748     +2311     
  Lines       55239   254096   +198857     
===========================================
+ Hits        19906   154565   +134659     
- Misses      35333    99531    +64198

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

Hi @zasdfgbnm!

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!

ngimel · 2020-10-30T22:21:27Z

+    @onlyOnCPUAndCUDA
+    @dtypesIfCUDA(*(torch.testing.get_all_fp_dtypes(include_bfloat16=AMPERE_OR_ROCM)))
    @dtypes(*(torch.testing.get_all_complex_dtypes() + [torch.float, torch.double]))
+    @tf32_on_and_off(0.05)


floating_and_complex_types() on the previous line

Is this commit missing?

It's not missing, after offline discussion, we decided that instead of testing on floating_and_complex_types(), we test on all fp and complex, and assertRaise for unsupported dtypes. So this comment no longer applies.

ngimel · 2020-10-30T22:23:06Z

+
+        for perm1, perm2 in product(permutations((0, 1, 2)), repeat=2):
+            for perm3 in permutations((0, 1)):
+                b1 = torch.randn(num_batches, M, N, dtype=dtype, device=device)


same as for baddbmm, please compare with numpy instead of this convoluted test logic. Also, if it makes sense to unify this test with baddbmm (at first glance it looks similar), please do so.

~~I think it's better to test separately. The reason is, the output of addbmm is 2D but baddbmm is 3D, so it is~~

for perm1, perm2 in product(permutations((0, 1, 2)), repeat=2): for perm3 in permutations((0, 1)):

vs

for perm1, perm2, perm3 in product(permutations((0, 1, 2)), repeat=3):

~~I don't know how these two can be combined easily.~~

Edit: maybe possible, I think I found a solution

Also, I don't think numpy has something like addbmm, the the difference is only torch.bmm(b1, b2).sum(0, False) vs numpy.matmul(b1, b2).sum(0, False), in this case, does it still make sense to change to numpy?

dr-ci · 2020-10-30T22:50:12Z

💊 CI failures summary and remediations

As of commit 781f4af (more details on the Dr. CI page):

1/2 failures introduced in this PR
1/2 broken upstream at merge base 2caa3bd since Nov 03

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

pytorch_windows_vs2019_py36_cuda10.1_test1 (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

[ FAILED ] TestQTensor.QuantizePerChannel4d

    Which is: 93 
[  FAILED  ] TestQTensor.QuantizePerChannel4d (0 ms) 
[ RUN      ] TestQTensor.QuantizePerChannel4dChannelsLast 
[       OK ] TestQTensor.QuantizePerChannel4dChannelsLast (14 ms) 
[----------] 7 tests from TestQTensor (21 ms total) 
 
[----------] Global test environment tear-down 
[==========] 7 tests from 1 test case ran. (21 ms total) 
[  PASSED  ] 6 tests. 
[  FAILED  ] 1 test, listed below: 
[  FAILED  ] TestQTensor.QuantizePerChannel4d 
 
 1 FAILED TEST 
"quantized_test" failed with exit code 1 
+ cleanup
+ retcode=1
+ set +x

🚧 1 ongoing upstream failure:

These were probably caused by upstream breakages that are not fixed yet:

pytorch_macos_10_13_py3_test since Nov 03
- 🔁 rerun

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 17 times.

zasdfgbnm · 2020-10-30T22:55:34Z

Broadcasting test added

zasdfgbnm · 2020-10-30T23:22:16Z

@ngimel I have resolved most of your comments:

Add broadcasting tests
unify addbmm and baddbmm
I moved baddbmm test to numpy

except that

I didn't move addbmm test to numpy, because we have to do .sum anyway

zasdfgbnm · 2020-11-03T00:15:40Z

@ngimel My CLA is resolved

…-tests

zasdfgbnm · 2020-11-03T18:57:18Z

this should be ready

mruberry · 2020-11-04T01:09:00Z


-        res2.addbmm_(b1, b2)
-        self.assertEqual(res2, res.sum(0, False))
+        getattr(res2, func + "_")(b1, b2)


Consider explicitly passing the variants you want (i.e. passing torch.foo and torch.Tensor.foo and torch.Tensor.foo_)

I can not, because I need a string for the name of the operator to assert on warning messages.

mruberry · 2020-11-04T01:11:10Z

+        self.assertEqual(res2, ref)
        res3.copy_(res2)

        with self.maybeWarnsRegex(


Reviewer's note (not for this PR): we need a way of always triggering these warnings. "maybeWarnsRegex" doesn't do anything, currently.

mruberry · 2020-11-04T01:20:30Z

+        if self.device_type == 'cpu':
+            is_supported = True
+            if dtype == torch.bfloat16:
+                self.precision = 1  # 43 vs 43.75


Reviewer's note: It'd be nicer if we supported per-dtype precision overrides (also atol and rtol overrides)

facebook-github-bot

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

facebook-github-bot · 2020-11-05T05:14:59Z

@ngimel merged this pull request in f19637e.

Summary: This is to satisfy the request at pytorch#42553 (comment). See also pytorch#47124 Pull Request resolved: pytorch#47079 Reviewed By: ejguan Differential Revision: D24735356 Pulled By: ngimel fbshipit-source-id: 122fceb4902658f350c2fd6f92455adadd0ec2a4

Expand the test of torch.addbmm

87f64a7

pytorchbot added the open source label Oct 29, 2020

zasdfgbnm mentioned this pull request Oct 29, 2020

Expand the test of torch.bmm on CUDA #47070

Closed

Expand test of torch.test_baddbmm

54b5669

zasdfgbnm mentioned this pull request Oct 29, 2020

Expand test of torch.test_baddbmm #47080

Closed

albanD requested a review from ngimel October 29, 2020 21:38

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

zasdfgbnm mentioned this pull request Oct 29, 2020

Port bmm and baddbmm from TH to ATen #42553

Closed

ngimel reviewed Oct 30, 2020

View reviewed changes

zasdfgbnm added 2 commits October 30, 2020 15:27

Merge branch 'master' of github.com:pytorch/pytorch into addbmm-tests

0dd51f0

floating_and_complex_types

78be046

Add broadcasting

025222c

zasdfgbnm added 5 commits October 30, 2020 16:06

unify

78b62ba

Merge branch 'baddbmm-tests' into addbmm-tests

24f8b35

merge baddbmm

7130c02

add broadcasting test

8fbb5a0

use numpy for test_baddbmm

6979f29

zasdfgbnm changed the title ~~Expand the test of torch.addbmm~~ Expand the test of torch.addbmm and torch.baddbmm Oct 30, 2020

zasdfgbnm added 2 commits October 30, 2020 18:17

Merge branch 'master' of github.com:pytorch/pytorch into addbmm-tests

a6fd956

Merge branch 'master' into addbmm-tests

1794ba6

facebook-github-bot added the cla signed label Nov 2, 2020

Merge branch 'master' of github.com:pytorch/pytorch into addbmm-tests

3b34411

ngimel reviewed Nov 3, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

ngimel reviewed Nov 3, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

Comment thread test/test_torch.py Outdated

zasdfgbnm added 2 commits November 2, 2020 18:59

save

8ad5c1c

Merge branch 'addbmm-tests' of github.com:pytorch/pytorch into addbmm…

4b592ab

…-tests

mruberry reviewed Nov 4, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

mruberry reviewed Nov 4, 2020

View reviewed changes

Merge branch 'master' of github.com:pytorch/pytorch into addbmm-tests

874cecf

mruberry reviewed Nov 4, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

mruberry reviewed Nov 4, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

ngimel reviewed Nov 4, 2020

View reviewed changes

Comment thread test/test_torch.py Outdated

zasdfgbnm added 3 commits November 3, 2020 18:15

save

0377f65

save

f6ab53b

Use make_tensor

781f4af

ngimel approved these changes Nov 4, 2020

View reviewed changes

facebook-github-bot reviewed Nov 4, 2020

View reviewed changes

facebook-github-bot closed this in f19637e Nov 5, 2020

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

zasdfgbnm deleted the addbmm-tests branch November 5, 2020 05:16

Conversation

zasdfgbnm commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

facebook-github-bot commented Oct 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zasdfgbnm Oct 30, 2020 • 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!

Choose a reason for hiding this comment

Uh oh!

dr-ci Bot commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 1 new failure recognized by patterns

pytorch_windows_vs2019_py36_cuda10.1_test1 (1/1)

🚧 1 ongoing upstream failure:

Uh oh!

zasdfgbnm commented Oct 30, 2020

Uh oh!

zasdfgbnm commented Oct 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zasdfgbnm commented Nov 3, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zasdfgbnm commented Nov 3, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zasdfgbnm commented Oct 29, 2020 •

edited

Loading

codecov Bot commented Oct 29, 2020 •

edited

Loading

zasdfgbnm Oct 30, 2020 •

edited

Loading

dr-ci Bot commented Oct 30, 2020 •

edited

Loading

zasdfgbnm commented Oct 30, 2020 •

edited

Loading