KEMBAR78
Fix FFT documentation examples and run doctests in the test suite by peterbell10 · Pull Request #60304 · pytorch/pytorch · GitHub
Skip to content

Conversation

@peterbell10
Copy link
Collaborator

Fixes #59514

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Jun 18, 2021

💊 CI failures summary and remediations

As of commit 4226688 (more details on the Dr. CI page and at hud.pytorch.org/pr/60304):


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

🕵️ 2 new failures recognized by patterns

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

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_build (1/2)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result.
CONFLICT (add/add): Merge conflict in .github/pytorch-circleci-labels.yml
Auto-merging .github/pytorch-circleci-labels.yml
CONFLICT (add/add): Merge conflict in .circleci/scripts/windows_cuda_install.sh
Auto-merging .circleci/scripts/windows_cuda_install.sh
CONFLICT (add/add): Merge conflict in .circleci/scripts/setup_ci_environment.sh
Auto-merging .circleci/scripts/setup_ci_environment.sh
CONFLICT (add/add): Merge conflict in .circleci/config.yml
Auto-merging .circleci/config.yml
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/windows_build_definitions.py
Auto-merging .circleci/cimodel/data/windows_build_definitions.py
Automatic merge failed; fix conflicts and then commit the result.


Exited with code exit status 1

See CircleCI build pytorch_linux_xenial_py3_6_gcc5_4_build (2/2)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result.
CONFLICT (add/add): Merge conflict in .github/pytorch-circleci-labels.yml
Auto-merging .github/pytorch-circleci-labels.yml
CONFLICT (add/add): Merge conflict in .circleci/scripts/windows_cuda_install.sh
Auto-merging .circleci/scripts/windows_cuda_install.sh
CONFLICT (add/add): Merge conflict in .circleci/scripts/setup_ci_environment.sh
Auto-merging .circleci/scripts/setup_ci_environment.sh
CONFLICT (add/add): Merge conflict in .circleci/config.yml
Auto-merging .circleci/config.yml
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/windows_build_definitions.py
Auto-merging .circleci/cimodel/data/windows_build_definitions.py
Automatic merge failed; fix conflicts and then commit the result.


Exited with code exit status 1


ci.pytorch.org: 1 failed


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.

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.

Cool! Thanks @peterbell10.

cc @pmeier, this is another good datapoint that maybe we should default check_strides to False

@facebook-github-bot
Copy link
Contributor

@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 5d476f5.

@pmeier
Copy link
Collaborator

pmeier commented Jun 24, 2021

@peterbell10 when #60637 is landed, you can drop the check_stride=False, since this will be the new default.

pmeier added a commit that referenced this pull request Jun 24, 2021
We now have ~three out of three~  four out of four datapoints that `check_stride` will be `partial`'ed to `False`:

- `torch` test suite: #58981 (comment)
- `torchvision` test suite: #56544 (comment)
- `kornia`: https://github.com/kornia/kornia/blob/9041c42b410e6a4bbb664c7134a120be80aa2265/test/utils.py#L25
- `torch.fft`: #60304 (review)

Given that the strides in most cases are in implementation detail, IMO we should change the default to `False`. In cases were matching strides is a requirement for closeness / equality it can always set to `True` manually.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Jun 24, 2021
We now have ~three out of three~  four out of four datapoints that `check_stride` will be `partial`'ed to `False`:

- `torch` test suite: #58981 (comment)
- `torchvision` test suite: #56544 (comment)
- `kornia`: https://github.com/kornia/kornia/blob/9041c42b410e6a4bbb664c7134a120be80aa2265/test/utils.py#L25
- `torch.fft`: #60304 (review)

Given that the strides in most cases are in implementation detail, IMO we should change the default to `False`. In cases were matching strides is a requirement for closeness / equality it can always set to `True` manually.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Jul 1, 2021
We now have ~three out of three~  four out of four datapoints that `check_stride` will be `partial`'ed to `False`:

- `torch` test suite: #58981 (comment)
- `torchvision` test suite: #56544 (comment)
- `kornia`: https://github.com/kornia/kornia/blob/9041c42b410e6a4bbb664c7134a120be80aa2265/test/utils.py#L25
- `torch.fft`: #60304 (review)

Given that the strides in most cases are in implementation detail, IMO we should change the default to `False`. In cases were matching strides is a requirement for closeness / equality it can always set to `True` manually.

[ghstack-poisoned]
pmeier added a commit that referenced this pull request Jul 1, 2021
We now have ~three out of three~  four out of four datapoints that `check_stride` will be `partial`'ed to `False`:

- `torch` test suite: #58981 (comment)
- `torchvision` test suite: #56544 (comment)
- `kornia`: https://github.com/kornia/kornia/blob/9041c42b410e6a4bbb664c7134a120be80aa2265/test/utils.py#L25
- `torch.fft`: #60304 (review)

Given that the strides in most cases are in implementation detail, IMO we should change the default to `False`. In cases were matching strides is a requirement for closeness / equality it can always set to `True` manually.

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Jul 7, 2021
Summary:
Pull Request resolved: #60637

We now have ~three out of three~  four out of four datapoints that `check_stride` will be `partial`'ed to `False`:

- `torch` test suite: #58981 (comment)
- `torchvision` test suite: #56544 (comment)
- `kornia`: https://github.com/kornia/kornia/blob/9041c42b410e6a4bbb664c7134a120be80aa2265/test/utils.py#L25
- `torch.fft`: #60304 (review)

Given that the strides in most cases are in implementation detail, IMO we should change the default to `False`. In cases were matching strides is a requirement for closeness / equality it can always set to `True` manually.

Test Plan: Imported from OSS

Reviewed By: ngimel

Differential Revision: D29556355

Pulled By: mruberry

fbshipit-source-id: 0029a44280d8f4369fbdb537dce3202eeee4b1d9
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.

irfft2 size issue for PyTorch 1.8.1

4 participants