Add meta device to generic device testing framework, skip NotImplementedError by ezyang · Pull Request #53682 · pytorch/pytorch

ezyang · 2021-03-10T04:06:41Z

Stack from ghstack:

Restore storage on meta tensors; increase meta coverage #53973 Add some trivial meta functions for commonly used inplace operations
Make storage access error NotImplementedError #53972 Make storage access error NotImplementedError
Compute type_equal() without reference to backend() #53823 Compute type_equal() without reference to backend()
Also propagate storage_access_should_throw_ when copying tensor metadata #53816 Also propagate storage_access_should_throw_ when copying tensor metadata
Fix restriding logic for structured kernels #53759 Fix restriding logic for structured kernels
Add meta device to generic device testing framework, skip NotImplementedError #53682 Add meta device to generic device testing framework, skip NotImplementedError
Add 'noarch' tests which only run in one CI config #53747 Add 'noarch' tests which only run in one CI config

With this, under the meta device, 101 tests passed and 16953 skipped.
It ain't much, but it's a start.

Some various bits and bobs:

NotImplementedError suppression at test level is implemented
in the same way as CUDA memory leak check, i.e., by wrapping
test methods and monkeypatching them back in.
I had to reimplement assertRaises/assertRaisesRegex from scratch to
ignore NotImplementedError when _ignore_not_implemented_error is True.
The implementation relies on a small amount of private API that hasn't
changed since 2010
expectedAlertNondeterministic doesn't really work so I skipped them
all; there's probably a way to do it better

I tested this using pytest --disable-warnings --tb=native -k meta --sw test/*.py and a pile of extra patches to make collection actually work
(lol).

Signed-off-by: Edward Z. Yang ezyang@fb.com

Differential Revision: D26955539

…tedError With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

…tedError With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: b4a1846 Pull Request resolved: #53682

facebook-github-bot · 2021-03-10T04:07:15Z

💊 CI failures summary and remediations

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

1/1 failures possibly* introduced in this PR
- 1/1 non-scanned failure(s)

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.

ezyang · 2021-03-10T04:44:04Z

Here is what is currently causing tests to skip:

            {'aten::_copy_from': 2167,
             'aten::arange.start_out': 24,
             'aten::as_strided': 8,
             'aten::eye.m_out': 45,
             'aten::fill_.Scalar': 138,
             'aten::linspace.out': 20,
             'aten::logspace.out': 18,
             'aten::normal_': 1757,
             'aten::random_': 9,
             'aten::random_.from': 4103,
             'aten::randperm.generator_out': 20,
             'aten::range.out': 2,
             'aten::take': 1,
             'aten::unfold': 1,
             'aten::uniform_': 4236,
             'aten::view': 4,
             'aten::zero_': 62,
             'torch.tensor': 1146})

computed by looking at the junit xml with

from junitparser import JUnitXml
import re
from collections import defaultdict
from pprint import pprint

xml = JUnitXml.fromfile('foo.xml')

result = defaultdict(int)

for suite in xml:
    for case in suite:
        if case.result:
            r = case.result[0]
            if r.type == "pytest.skip" and r.message.startswith("not implemented"):
                m = re.search(r"Could not run '([^']+)'", r.text)
                if m:
                    op = m.group(1)
                elif "torch.tensor" in r.text:
                    op = "torch.tensor"
                else:
                    continue
                result[op] += 1

pprint(result)

…NotImplementedError" With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

…tedError With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 6f86730 Pull Request resolved: #53682

mruberry · 2021-03-10T16:04:40Z

Adding a meta device seems cool. How time consuming are meta tests? Would it make sense to limit them to only certain builds?

Follow-up question: is there a planned announcement regarding meta tests? What should people do if a meta test fails? Do they need to call you in to unblock the PR, or do you recommend skipping the meta device and filing an issue, for example?

I made a few inline comments. Mostly suggestions for additional comments, but a sanity check on subclassing unittest.case._AssertRaisesContext, too.

ezyang · 2021-03-10T19:36:01Z

How time consuming are meta tests?

On an optimized build of PyTorch, it still takes a few minutes to run them all because Python is slow :( (according to a XML file I have lying around, it took just shy of 7min to run all test with "meta" in their name). 7min isn't half an hour though, so maybe it's OK? I think it would be pretty OK to only run these on one build, though, because (1) a regular CPU kernel will still exercise all this and (2) the meta kernels aren't very platform sensitive (unlike other sense)

mruberry · 2021-03-10T19:40:07Z

How time consuming are meta tests?

On an optimized build of PyTorch, it still takes a few minutes to run them all because Python is slow :( (according to a XML file I have lying around, it took just shy of 7min to run all test with "meta" in their name). 7min isn't half an hour though, so maybe it's OK? I think it would be pretty OK to only run these on one build, though, because (1) a regular CPU kernel will still exercise all this and (2) the meta kernels aren't very platform sensitive (unlike other sense)

Limiting them to one build would be preferable if possibly. 7 minutes isn't so bad but 7 minutes on every test build adds up.

I also forgot to mention that some tests use the @onlyOnCPUAndCUDA decorator, but those tests probably want to be @onlyOnCPUAndCUDAAndMeta now. Should we change the decorators so that if CPU is run then Meta is run, too, and require explicitly skipping meta? So @onlyCPU would actually be @onlyCPUAndMeta, for example? And to actually run exclusively on the CPU the test would need to have both the @onlyCPU decorator and the @skipMeta decorator?

ezyang · 2021-03-10T19:41:49Z

Follow-up question: is there a planned announcement regarding meta tests? What should people do if a meta test fails? Do they need to call you in to unblock the PR, or do you recommend skipping the meta device and filing an issue, for example?

If I've done my job right it should be obvious how to fix these problems (and you should be unlikely to break the meta test if you aren't working on anything meta), but yeah, we can do a short post about it.

ezyang · 2021-03-10T19:57:12Z

I also forgot to mention that some tests use the @onlyOnCPUAndCUDA decorator, but those tests probably want to be @onlyOnCPUAndCUDAAndMeta now. Should we change the decorators so that if CPU is run then Meta is run, too, and require explicitly skipping meta? So @onlyCPU would actually be @onlyCPUAndMeta, for example? And to actually run exclusively on the CPU the test would need to have both the @onlyCPU decorator and the @skipMeta decorator?

I think we need to sit down and have a serious design discussion about how to organize these decorators.

…NotImplementedError" With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

…tedError With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 27d9770 Pull Request resolved: #53682

…NotImplementedError" With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26955539](https://our.internmc.facebook.com/intern/diff/D26955539) [ghstack-poisoned]

mruberry · 2021-03-12T10:01:14Z

Offline discussion with @ezyang is tracked in #53849.

mruberry

LGTM!

…NotImplementedError" With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26955539](https://our.internmc.facebook.com/intern/diff/D26955539) [ghstack-poisoned]

facebook-github-bot · 2021-03-15T03:41:47Z

@ezyang merged this pull request in c2f41b6.

…tedError (pytorch#53682) Summary: Pull Request resolved: pytorch#53682 With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> Test Plan: Imported from OSS Reviewed By: mruberry Differential Revision: D26955539 Pulled By: ezyang fbshipit-source-id: ac21c8734562497fdcca3b614a28010bc4c03d74

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> Differential Revision: [D35010278](https://our.internmc.facebook.com/intern/diff/D35010278) [ghstack-poisoned]

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> ghstack-source-id: d0ea328 Pull Request resolved: #74468

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> Differential Revision: [D35010278](https://our.internmc.facebook.com/intern/diff/D35010278) [ghstack-poisoned]

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> ghstack-source-id: 2442622 Pull Request resolved: #74468

After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> Pull Request resolved: #74468 Approved by: https://github.com/mruberry

Summary: After discussion with Can Balioglu, we have concluded that #53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> Pull Request resolved: #74468 Approved by: https://github.com/mruberry Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/65329f4fac8fb22318b7a3eb115e9da207d8d41a Reviewed By: bigfootjon Differential Revision: D35193490 Pulled By: atalman fbshipit-source-id: 6e6bae34c075e60c6a84936f812e178bf88784cd

…tedError (pytorch#53682) Summary: Pull Request resolved: pytorch#53682 With this, under the meta device, 101 tests passed and 16953 skipped. It ain't much, but it's a start. Some various bits and bobs: - NotImplementedError suppression at test level is implemented in the same way as CUDA memory leak check, i.e., by wrapping test methods and monkeypatching them back in. - I had to reimplement assertRaises/assertRaisesRegex from scratch to ignore NotImplementedError when _ignore_not_implemented_error is True. The implementation relies on a small amount of private API that hasn't changed since 2010 - expectedAlertNondeterministic doesn't really work so I skipped them all; there's probably a way to do it better I tested this using `pytest --disable-warnings --tb=native -k meta --sw test/*.py` and a pile of extra patches to make collection actually work (lol). Signed-off-by: Edward Z. Yang <ezyang@fb.com> Test Plan: Imported from OSS Reviewed By: mruberry Differential Revision: D26955539 Pulled By: ezyang fbshipit-source-id: ac21c8734562497fdcca3b614a28010bc4c03d74

After discussion with Can Balioglu, we have concluded that pytorch#53682 , while clever, is more trouble than it is worth. The main problem is that whenever someone adds support for new meta tensors, they then get dozens of new test case failures, because tests that were previously halted by lack of support for an operator on meta tensors, now have gotten further and hit some logic which expects to be able to, e.g., pull out a real value from a tensor (which clearly doesn't work). This is very annoying and time consuming! Most of these tests aren't written with meta device in mind, and it's not a good use of time to try to make them more generic. The plan on record is to switch meta testing to OpInfo, but that patch will take some time to prepare for now I want to stem the bleeding. I don't think we're at high risk for regressions here because meta tensors mostly share logic with their regular brethren. Signed-off-by: Edward Z. Yang <ezyangfb.com> Pull Request resolved: pytorch#74468 Approved by: https://github.com/mruberry

This was referenced Mar 10, 2021

Convert a few more checks to raise NotImplementedError #53610

Closed

Fix inaccurate dispatch table for fill_ #53611

Closed

Marginally improve pytest collection for top-level test files #53617

Closed

Make storage access throw for meta tensors #53681

Closed

facebook-github-bot added the cla signed label Mar 10, 2021

ezyang requested review from bdhirsh, bhosmer and mruberry March 10, 2021 04:08