Skip to content

[RELAND] Add __torch_function__ benchmarks#36138

Closed
hameerabbasi wants to merge 6 commits intomasterfrom
ci-all/torch-function-benchmarks
Closed

[RELAND] Add __torch_function__ benchmarks#36138
hameerabbasi wants to merge 6 commits intomasterfrom
ci-all/torch-function-benchmarks

Conversation

@hameerabbasi
Copy link
Copy Markdown
Collaborator

@hameerabbasi hameerabbasi commented Apr 7, 2020

Re-land of #35530 and #34645

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Apr 7, 2020

💊 CircleCI build failures summary and remediations

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


  • 2/2 failures introduced in this PR

2 jobs timed out:

  • pytorch_linux_xenial_cuda9_2_cudnn7_py3_gcc7_test
  • pytorch_linux_xenial_cuda9_cudnn7_py3_test

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 on the GitHub issue tracker.

See how this bot performed.

This comment has been revised 21 times.

@hameerabbasi hameerabbasi force-pushed the ci-all/torch-function-benchmarks branch from e69ae62 to bf48df6 Compare April 7, 2020 13:47
Copy link
Copy Markdown
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

waiting on CI. For reference, can you cross reference the previous time we landed this PR?

@ezyang ezyang changed the title Add __torch_function__ benchmarks [RELAND] Add __torch_function__ benchmarks Apr 7, 2020
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@hameerabbasi
Copy link
Copy Markdown
Collaborator Author

Out of the three failures:

  • Bazel is broken in upstream, and there's a merge conflict if I do not merge that commit.
  • pytorch_linux_xenial_py3_clang5_asan_test: Introduces UB, but I'm not touching any C/C++ code.
  • The pr/caffe2-py3.6-devtoolset7-rocmrpm-centos7-test failure is a timeout:
14:13:52 [ RUN      ] UtilityOpGPUTest.testReshapeWithScalar
17:12:51 Build timed out (after 180 minutes). Marking the build as failed.

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Apr 8, 2020

The ASAN failure is very clearly related to the code you added though, because it's reporting UB when running bench.py. You're probably dividing by zero from userland and that is triggering UBSAN...

Comment on lines +19 to +20
bench_time = float(torch.min(torch.Tensor(bench_times))) / 1000
bench_std = float(torch.std(torch.Tensor(bench_times))) / 1000
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@ezyang Only division happens here, and it's by 1000, not 0. Elsewhere I'm only doing torch.add.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Got it, torch.std has UB with a length-1 Tensor.

@hameerabbasi hameerabbasi force-pushed the ci-all/torch-function-benchmarks branch from 55e32c8 to 79bfa6b Compare April 9, 2020 10:56
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@ezyang
Copy link
Copy Markdown
Contributor

ezyang commented Apr 9, 2020

If you think std shouldn't UB when passed a one length tensor (a view I'm sympathetic with), file a bug report!

@hameerabbasi hameerabbasi force-pushed the ci-all/torch-function-benchmarks branch from 1823929 to dd63c44 Compare April 9, 2020 14:38
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ezyang merged this pull request in 7c825ba.

ashishfarmer pushed a commit to ashishfarmer/pytorch that referenced this pull request Apr 13, 2020
Summary:
Re-land of pytorch#35530 and pytorch#34645
Pull Request resolved: pytorch#36138

Differential Revision: D20893770

Pulled By: ezyang

fbshipit-source-id: 75ab688a086f5fb87412a853df5246c0c39704ca
@facebook-github-bot facebook-github-bot deleted the ci-all/torch-function-benchmarks branch July 13, 2020 17:54
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Re-land of pytorch#35530 and pytorch#34645
Pull Request resolved: pytorch#36138

Differential Revision: D20893770

Pulled By: ezyang

fbshipit-source-id: 75ab688a086f5fb87412a853df5246c0c39704ca
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.

5 participants