Add 'noarch' tests which only run in one CI config by ezyang · Pull Request #53747 · pytorch/pytorch

ezyang · 2021-03-10T21:26:55Z

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

Fixes #53743

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

Differential Revision: D26971343

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

facebook-github-bot · 2021-03-10T21:27:02Z

💊 CI failures summary and remediations

As of commit cd546d4 (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.

mruberry · 2021-03-11T00:09:35Z

    return wrapper


+def noarchTest(fn):


Comment for this decorator explaining what a "NoArch" test is and how the decorator is intended to be used?

Edit: removed a second question answered by the corresponding issue.

mruberry

This looks correct up to name bikeshedding (which we should probably use the issue for)

mruberry · 2021-03-11T00:16:58Z

Test failures appear to be in the base.

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

janeyx99 · 2021-03-11T15:33:22Z

This seems like a functional way to get the intended behavior, but I have some uneasiness regarding just picking a config and having that somewhat "silently" be designated the noarch config.

Concerns:

Despite the WARNING and the comments throughout the code regarding noarch tests, it is still possible for someone to mess up and delete the config without moving the noarch tests, and we have no fast obvious way of detecting if that happens.
If for whatever reason this build/test config was corrupted and couldn't run for a commit, it isn't immediately clear that the noarch tests may not have been run at all.

Potential solution that may alleviate the above:
I think I'd be more comfortable if we had a designated test config that only ran noarch tests (similar to how we have a separate slow tests CI job). The test config can stem from the pytorch-linux-bionic-py3.6-clang9 build. If we have "noarch" in the test CI job name/build environment, it'd be more clear when the job gets removed from CI later on. It'd also isolate the noarch tests from other configs so we'd get less mixed signal.

janeyx99 · 2021-03-11T15:44:59Z

  export BUILD_SPLIT_CUDA=ON
 fi

+if [[ "$BUILD_ENVIRONMENT" == *pytorch-linux-bionic-py3.6-clang9* ]]; then


An advantage to using the potential solution I mentioned in the other comment is we'd be able to have something more clear here, like:

Suggested change

if [[ "$BUILD_ENVIRONMENT" == *pytorch-linux-bionic-py3.6-clang9* ]]; then

if [[ "$BUILD_ENVIRONMENT" == *noarch* ]]; then

janeyx99 · 2021-03-11T15:45:42Z

                    self.assertEqual(output3, output1)
                    self.assertEqual(output3, output2)

+        @noarchTest


Will there also be a PR going through every test in the codebase so far to mark some as noarch or will it be more of a gradual "from now on" thing?

It'll be as needed. The main motivating use case for this is I'm adding 17k new tests in #53682 and I don't want them to run on all CI runs.

janeyx99 · 2021-03-11T15:48:11Z

+    @wraps(fn)
+    def wrapper(*args, **kwargs):
+        if TEST_SKIP_NOARCH:
+            raise unittest.SkipTest("test is noarch but we are skipping noarch tests due to TEST_SKIP_NOARCH")


nitty nit (the "but" was a little confusing but it's ok the way it is now too if you want to land)

Suggested change

raise unittest.SkipTest("test is noarch but we are skipping noarch tests due to TEST_SKIP_NOARCH")

raise unittest.SkipTest("test is noarch: we are skipping noarch tests due to TEST_SKIP_NOARCH")

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

ezyang · 2021-03-11T16:47:56Z

@janeyx99 okey dokey, the CI job has been renamed (I thought this would be painful but actually we have ignored for docker tags so it's fine)

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 3542fbc Pull Request resolved: #53747

janeyx99 · 2021-03-11T17:11:44Z

            python_version = fc.find_prop("pyver")

+        if is_noarch:
+            parms_list_ignored_for_docker_image.append("noarch")


Suggested change

parms_list_ignored_for_docker_image.append("noarch")

restrict_phases = ["build"] if restrict_phases is None else restrict_phases

restrict_phases.extend(["test", "noarch_test"])

I believe this code would not change the name of the existing build but instead would create a separate CI test job that only runs noarch tests, which would help with isolating the tests, if that is to be desired

I think we don't want this change? I don't want to split out the noarch tests into one config that only runs them, because I don't have any evidence that sharding them separately would be a benefit (noarch is much faster than normal tests so it would be super lopsided).

Yea, that's fine in that case.

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

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

@ezyang merged this pull request in d47d246.

Summary: Pull Request resolved: pytorch#53747 Fixes pytorch#53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Test Plan: Imported from OSS Reviewed By: albanD Differential Revision: D26971343 Pulled By: ezyang fbshipit-source-id: cee7aa10063ae674f741406a3af830e4b4f128df

Add 'noarch' tests which only run in one CI config

d0e11ef

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

This was referenced Mar 10, 2021

Make storage access throw for meta tensors #53681

Closed

Add meta device to generic device testing framework, skip NotImplementedError #53682

Closed

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

ezyang requested review from janeyx99, malfet and mruberry March 10, 2021 21:36

ezyang mentioned this pull request Mar 10, 2021

Fix restriding logic for structured kernels #53759

Closed

mruberry reviewed Mar 11, 2021

View reviewed changes

mruberry approved these changes Mar 11, 2021

View reviewed changes

Update on "Add 'noarch' tests which only run in one CI config"

a1f0a5d

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]

janeyx99 reviewed Mar 11, 2021

View reviewed changes

Update on "Add 'noarch' tests which only run in one CI config"

7c050c1

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

ezyang mentioned this pull request Mar 11, 2021

Also propagate storage_access_should_throw_ when copying tensor metadata #53816

Closed

Update on "Add 'noarch' tests which only run in one CI config"

63a6fe8

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

ezyang mentioned this pull request Mar 11, 2021

Compute type_equal() without reference to backend() #53823

Closed

Update on "Add 'noarch' tests which only run in one CI config"

01027a8

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

Update on "Add 'noarch' tests which only run in one CI config"

0ea3425

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

ezyang added a commit that referenced this pull request Mar 11, 2021

Add 'noarch' tests which only run in one CI config

5df5535

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 3542fbc Pull Request resolved: #53747

janeyx99 approved these changes Mar 11, 2021

View reviewed changes

malfet approved these changes Mar 11, 2021

View reviewed changes

Update on "Add 'noarch' tests which only run in one CI config"

c7b6db9

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

This was referenced Mar 14, 2021

Make storage access error NotImplementedError #53972

Closed

Restore storage on meta tensors; increase meta coverage #53973

Closed

Update on "Add 'noarch' tests which only run in one CI config"

861a4e4

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

Update on "Add 'noarch' tests which only run in one CI config"

cd546d4

Fixes #53743 Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D26971343](https://our.internmc.facebook.com/intern/diff/D26971343) [ghstack-poisoned]

facebook-github-bot closed this in d47d246 Mar 15, 2021

facebook-github-bot added the Merged label Mar 15, 2021

facebook-github-bot deleted the gh/ezyang/940/head branch March 18, 2021 14:16

janeyx99 mentioned this pull request Apr 6, 2021

Move distributed tests to single CI configuration #55173

Closed

	if [[ "$BUILD_ENVIRONMENT" == pytorch-linux-bionic-py3.6-clang9 ]]; then
	if [[ "$BUILD_ENVIRONMENT" == noarch ]]; then

	raise unittest.SkipTest("test is noarch but we are skipping noarch tests due to TEST_SKIP_NOARCH")
	raise unittest.SkipTest("test is noarch: we are skipping noarch tests due to TEST_SKIP_NOARCH")

	parms_list_ignored_for_docker_image.append("noarch")
	restrict_phases = ["build"] if restrict_phases is None else restrict_phases
	restrict_phases.extend(["test", "noarch_test"])

Conversation

ezyang commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Mar 10, 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 Mar 11, 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!

mruberry left a comment

Choose a reason for hiding this comment

Uh oh!

mruberry commented Mar 11, 2021

Uh oh!

janeyx99 commented Mar 11, 2021

Uh oh!

janeyx99 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

janeyx99 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

ezyang Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

janeyx99 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

ezyang commented Mar 11, 2021

Uh oh!

janeyx99 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

ezyang Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

janeyx99 Mar 11, 2021

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ezyang commented Mar 10, 2021 •

edited

Loading

facebook-github-bot commented Mar 10, 2021 •

edited

Loading

mruberry Mar 11, 2021 •

edited

Loading