Skip to content

Conversation

@pmeier
Copy link
Collaborator

@pmeier pmeier commented Apr 19, 2021

Stack from ghstack:

Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with @onlyCPU, this factors out all tests that actually need another device into a separate test case.

Differential Revision: D28247529

Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 19, 2021

💊 CI failures summary and remediations

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


None of the CI failures appear to be your fault 💚



❄️ 1 failure tentatively classified as flaky

but reruns have not yet been triggered to confirm:

See CircleCI build pytorch_windows_vs2019_py36_cpu_build (1/1)

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

requests.exceptions.ChunkedEncodingError: ("Con...ly closed by the remote host', None, 10054, None))
      File "C:\Jenkins\Miniconda3\lib\site-packages\requests\sessions.py", line 543, in get
        return self.request('GET', url, **kwargs)
      File "C:\Jenkins\Miniconda3\lib\site-packages\requests\sessions.py", line 530, in request
        resp = self.send(prep, **send_kwargs)
      File "C:\Jenkins\Miniconda3\lib\site-packages\requests\sessions.py", line 685, in send
        r.content
      File "C:\Jenkins\Miniconda3\lib\site-packages\requests\models.py", line 829, in content
        self._content = b''.join(self.iter_content(CONTENT_CHUNK_SIZE)) or b''
      File "C:\Jenkins\Miniconda3\lib\site-packages\requests\models.py", line 754, in generate
        raise ChunkedEncodingError(e)
    requests.exceptions.ChunkedEncodingError: ("Connection broken: ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None)", ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None))

`$ C:\Jenkins\Miniconda3\Scripts\conda-script.py install -y -q -c conda-forge libuv=1.39`

  environment variables:
                 CIO_TEST=<not set>
       CMAKE_INCLUDE_PATH=C:\Users\circleci\project\build\win_tmp\mkl\include
        CONDA_DEFAULT_ENV=base
                CONDA_EXE=C:\Jenkins\Miniconda3\condabin\..\Scripts\conda.exe
               CONDA_EXES="C:\Jenkins\Miniconda3\condabin\..\Scripts\conda.exe"
         CONDA_PARENT_DIR=C:\Jenkins

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.

Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Apr 19, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: af973ee
Pull Request resolved: #56365
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. thanks for the prompt follow up @pmeier

Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
@mruberry
Copy link
Collaborator

This refactor PR seems unnecessary but I made some comments if we really want to pursue it as part of this stack

pmeier added 5 commits April 20, 2021 13:50
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
pmeier added a commit to pmeier/pytorch that referenced this pull request Apr 29, 2021
Follow-up to pytorch#54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 7a8d587
Pull Request resolved: pytorch#56365
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Apr 30, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 5af6316
Pull Request resolved: #56365
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request May 3, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 2e4d3c6
Pull Request resolved: #56365
Copy link
Collaborator

@mruberry mruberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks really good, @pmeier; just a couple suggestions

@mruberry
Copy link
Collaborator

mruberry commented May 6, 2021

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

Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

Differential Revision: [D28247529](https://our.internmc.facebook.com/intern/diff/D28247529)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request May 6, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 73a18c1
Pull Request resolved: #56365
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

Differential Revision: [D28247529](https://our.internmc.facebook.com/intern/diff/D28247529)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request May 9, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: de422d7
Pull Request resolved: #56365
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

Differential Revision: [D28247529](https://our.internmc.facebook.com/intern/diff/D28247529)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request May 9, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 8bffc66
Pull Request resolved: #56365
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

Differential Revision: [D28247529](https://our.internmc.facebook.com/intern/diff/D28247529)

[ghstack-poisoned]
pmeier added a commit that referenced this pull request May 11, 2021
Follow-up to #54784 (comment). Instead of having one large testcase where most methods are decorated with `@onlyCPU`, this factors out all tests that actually need another device into a separate test case.

ghstack-source-id: 61af033
Pull Request resolved: #56365
@mruberry
Copy link
Collaborator

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

@facebook-github-bot
Copy link
Contributor

@mruberry merged this pull request in 8824f49.

@facebook-github-bot facebook-github-bot deleted the gh/pmeier/10/head branch May 15, 2021 14:23
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
)

Summary:
Pull Request resolved: pytorch#56365

Follow-up to pytorch#54784 (comment). Instead of having one large testcase where most methods are decorated with `onlyCPU`, this factors out all tests that actually need another device into a separate test case.

Test Plan: Imported from OSS

Reviewed By: walterddr, albanD

Differential Revision: D28247529

Pulled By: mruberry

fbshipit-source-id: 946e7694b70e736941565f29b5dd459ed7fbca4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants