Fix fft slow tests by zasdfgbnm · Pull Request #50435 · pytorch/pytorch

zasdfgbnm · 2021-01-12T17:28:07Z

The failure is:

______________________________________________________________________________________________________ TestCommonCUDA.test_variant_consistency_jit_fft_rfft_cuda_float64 _______________________________________________________________________________________________________
../.local/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py:889: in wrapper
    method(*args, **kwargs)
../.local/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py:889: in wrapper
    method(*args, **kwargs)
../.local/lib/python3.9/site-packages/torch/testing/_internal/common_device_type.py:267: in instantiated_test
    if op is not None and op.should_skip(generic_cls.__name__, name,
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <torch.testing._internal.common_methods_invocations.SpectralFuncInfo object at 0x7f7375f9b550>, cls_name = 'TestCommon', test_name = 'test_variant_consistency_jit', device_type = 'cuda', dtype = torch.float64

    def should_skip(self, cls_name, test_name, device_type, dtype):
>       for si in self.skips:
E       TypeError: 'NoneType' object is not iterable

../.local/lib/python3.9/site-packages/torch/testing/_internal/common_methods_invocations.py:186: TypeError

facebook-github-bot · 2021-01-12T17:28:18Z

💊 CI failures summary and remediations

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

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI 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.

This comment has been revised 4 times.

mruberry · 2021-01-12T18:04:05Z

Would you elaborate on what's going on here, @zasdfgbnm?

cc @peterbell10

It seems like this line should be preventing skips from becoming None?

zasdfgbnm · 2021-01-12T18:10:25Z

When I run with PYTORCH_TEST_WITH_SLOW=1, the test_ops.py is failing with the above error for all these fft ops. It seems that nobody is running slow tests, so nobody found this failure. And this failure is because in the __init__ function, it should replace the skips=None with skips=[], regardless of the TEST_WITH_SLOW. But currently, it is only doing so if not TEST_WITH_SLOW, which is wrong.

To reproduce the failure, you can do

export PYTORCH_TEST_WITH_SLOW=1
python test/test_ops.py -v

mruberry

Nice fix, thanks @zasdfgbnm.

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-13T09:35:27Z

@mruberry merged this pull request in 4a3a378.

malfet · 2021-01-13T15:39:47Z

It seems that nobody is running slow tests, so nobody found this failure.

Hmm, slow test should be executed by slow_test config, see pytorch_linux_xenial_cuda10_2_cudnn7_py3_slow_test for example, though I wonder why failures is not reported back

peterbell10 · 2021-01-13T17:07:10Z

Ah, I think I see this issue. The CI is running with PYTORCH_TEST_SKIP_FAST but the test was never marked as slow, so it gets skipped on every CI run. I think OpInfo needs a way to directly add the @slowTest decorator to a particular test.

@mruberry I'm thinking either extend the SkipInfo class to handle slow tests specifically. Or, introduce a mechanism to add a set of decorators to a particular test. That could either replace SkipInfo, or coexist with it. What do you think?

mruberry · 2021-01-13T17:55:55Z

I could see either approach work. If SkipInfo can be extended in a natural way, like there's just a active_if=no_slow_tests option, that sounds good. If there are other scenarios where we need to apply decorators to specific tests then I would create a separate mechanism, although I cannot think of any at the moment.

peterbell10 · 2021-01-13T18:53:43Z

I'm leaning towards creating a DectorateInfo class that has a decorators argument, like the main OpInfo argument. The decorators get added while instantiating the test, just before where the test is currently wrapped in a functor that queries op.should_skip(...). Then optionally, SkipInfo can be reimplemented as a DecorateInfo that sets decorators = skipIf(true).

Adding the slowTest decorator requires this same logic anyway, so it seems reasonable to provide a generic interface in case it's needed elsewhere.

If that sounds alright, I'll create a PR.

mruberry · 2021-01-13T18:54:58Z

Sounds good.

…0501) Summary: Follow up to #50435 I have confirmed this works by running ``` pytest test_ops.py -k test_fn_gradgrad_fft` ``` with normally and with `PYTORCH_TEST_WITH_SLOW=1 PYTORCH_TEST_SKIP_FAST=1`. In the first case all tests are skipped, in the second they all run as they should. Pull Request resolved: #50501 Reviewed By: ezyang Differential Revision: D25956416 Pulled By: mruberry fbshipit-source-id: c896a8cec5f19b8ffb9b168835f3743b6986dad7

Summary: The failure is: ``` ______________________________________________________________________________________________________ TestCommonCUDA.test_variant_consistency_jit_fft_rfft_cuda_float64 _______________________________________________________________________________________________________ ../.local/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py:889: in wrapper method(*args, **kwargs) ../.local/lib/python3.9/site-packages/torch/testing/_internal/common_utils.py:889: in wrapper method(*args, **kwargs) ../.local/lib/python3.9/site-packages/torch/testing/_internal/common_device_type.py:267: in instantiated_test if op is not None and op.should_skip(generic_cls.__name__, name, _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <torch.testing._internal.common_methods_invocations.SpectralFuncInfo object at 0x7f7375f9b550>, cls_name = 'TestCommon', test_name = 'test_variant_consistency_jit', device_type = 'cuda', dtype = torch.float64 def should_skip(self, cls_name, test_name, device_type, dtype): > for si in self.skips: E TypeError: 'NoneType' object is not iterable ../.local/lib/python3.9/site-packages/torch/testing/_internal/common_methods_invocations.py:186: TypeError ``` Pull Request resolved: pytorch#50435 Reviewed By: izdeby Differential Revision: D25886650 Pulled By: mruberry fbshipit-source-id: 722a45247dc79be86858306cd1b51b0a63df8b37

…torch#50501) Summary: Follow up to pytorch#50435 I have confirmed this works by running ``` pytest test_ops.py -k test_fn_gradgrad_fft` ``` with normally and with `PYTORCH_TEST_WITH_SLOW=1 PYTORCH_TEST_SKIP_FAST=1`. In the first case all tests are skipped, in the second they all run as they should. Pull Request resolved: pytorch#50501 Reviewed By: ezyang Differential Revision: D25956416 Pulled By: mruberry fbshipit-source-id: c896a8cec5f19b8ffb9b168835f3743b6986dad7

Fix fft slow tests

cc8a796

facebook-github-bot added the cla signed label Jan 12, 2021

zasdfgbnm requested a review from mruberry January 12, 2021 17:28

pytorchbot added the open source label Jan 12, 2021

mruberry requested a review from peterbell10 January 12, 2021 18:04

mruberry added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 12, 2021

mruberry approved these changes Jan 12, 2021

View reviewed changes

facebook-github-bot reviewed Jan 12, 2021

View reviewed changes

facebook-github-bot closed this in 4a3a378 Jan 13, 2021

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

malfet deleted the slow-test-fix branch January 13, 2021 15:40

peterbell10 mentioned this pull request Jan 13, 2021

OpInfo: Add DecorateInfo class similar to SkipInfo for decorators #50501

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix fft slow tests#50435

Fix fft slow tests#50435
zasdfgbnm wants to merge 1 commit intomasterfrom
slow-test-fix

zasdfgbnm commented Jan 12, 2021

Uh oh!

facebook-github-bot commented Jan 12, 2021 •

edited

Loading

Uh oh!

mruberry commented Jan 12, 2021

Uh oh!

zasdfgbnm commented Jan 12, 2021 •

edited

Loading

Uh oh!

mruberry left a comment

Uh oh!

facebook-github-bot left a comment

Uh oh!

facebook-github-bot commented Jan 13, 2021

Uh oh!

malfet commented Jan 13, 2021 •

edited

Loading

Uh oh!

peterbell10 commented Jan 13, 2021 •

edited

Loading

Uh oh!

mruberry commented Jan 13, 2021

Uh oh!

peterbell10 commented Jan 13, 2021

Uh oh!

mruberry commented Jan 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

zasdfgbnm commented Jan 12, 2021

Uh oh!

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

Uh oh!

💊 CI failures summary and remediations

Uh oh!

mruberry commented Jan 12, 2021

Uh oh!

zasdfgbnm commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jan 13, 2021

Uh oh!

malfet commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peterbell10 commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mruberry commented Jan 13, 2021

Uh oh!

peterbell10 commented Jan 13, 2021

Uh oh!

mruberry commented Jan 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

facebook-github-bot commented Jan 12, 2021 •

edited

Loading

zasdfgbnm commented Jan 12, 2021 •

edited

Loading

malfet commented Jan 13, 2021 •

edited

Loading

peterbell10 commented Jan 13, 2021 •

edited

Loading