TensorIteratorConfig: Check memory overlap by default #43422

peterbell10 · 2020-08-21T19:55:37Z

Stack from ghstack:

Check for internal memory overlap in some indexing-type functions #43423 Check for internal memory overlap in some indexing-type functions
TensorIteratorConfig: Check memory overlap by default #43422 TensorIteratorConfig: Check memory overlap by default

Differential Revision: D23298653

[ghstack-poisoned]

dr-ci · 2020-08-21T20:04:08Z

💊 CI failures summary and remediations

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

2/2 failures introduced in this PR

🕵️ 2 new failures recognized by patterns

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

pytorch_linux_xenial_py3_6_gcc5_4_ge_config_simple_test (1/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Aug 31 19:00:58 ERROR:sccache::server: Compilation failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp: In function \'int main()\':\n/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp:2:23: error: expected \';\' before \'}\' token\n int main() { return 0 }\n                       ^\n" }

Aug 31 19:00:58 Traceback (most recent call last): 
Aug 31 19:00:58   File "test/run_test.py", line 734, in <module> 
Aug 31 19:00:58     main() 
Aug 31 19:00:58   File "test/run_test.py", line 717, in main 
Aug 31 19:00:58     raise RuntimeError(err_message) 
Aug 31 19:00:58 RuntimeError: test_type_hints failed! 
Aug 31 19:00:58 + cleanup 
Aug 31 19:00:58 + retcode=1 
Aug 31 19:00:58 + set +x 
Aug 31 19:00:58 =================== sccache compilation log =================== 
Aug 31 19:00:58 ERROR:sccache::server: Compilation failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp: In function \'int main()\':\n/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp:2:23: error: expected \';\' before \'}\' token\n int main() { return 0 }\n                       ^\n" } 
Aug 31 19:00:58  
Aug 31 19:00:58 =========== If your build fails, please take a look at the log above for possible reasons =========== 
Aug 31 19:00:58 Compile requests                 0 
Aug 31 19:00:58 Compile requests executed        0 
Aug 31 19:00:58 Cache hits                       0 
Aug 31 19:00:58 Cache misses                     0 
Aug 31 19:00:58 Cache timeouts                   0 
Aug 31 19:00:58 Cache read errors                0 
Aug 31 19:00:58 Forced recaches                  0 
Aug 31 19:00:58 Cache write errors               0

pytorch_linux_xenial_py3_6_gcc5_4_test (2/2)

Step: "Run tests" (full log | diagnosis details | 🔁 rerun)

Aug 31 19:01:33 ERROR:sccache::server: Compilation failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp: In function \'int main()\':\n/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp:2:23: error: expected \';\' before \'}\' token\n int main() { return 0 }\n                       ^\n" }

Aug 31 19:01:33 Traceback (most recent call last): 
Aug 31 19:01:33   File "test/run_test.py", line 734, in <module> 
Aug 31 19:01:33     main() 
Aug 31 19:01:33   File "test/run_test.py", line 717, in main 
Aug 31 19:01:33     raise RuntimeError(err_message) 
Aug 31 19:01:33 RuntimeError: test_type_hints failed! 
Aug 31 19:01:33 + cleanup 
Aug 31 19:01:33 + retcode=1 
Aug 31 19:01:33 + set +x 
Aug 31 19:01:33 =================== sccache compilation log =================== 
Aug 31 19:01:33 ERROR:sccache::server: Compilation failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "", stderr: "/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp: In function \'int main()\':\n/var/lib/jenkins/.cache/torch_extensions/test_compilation_error_formatting/main.cpp:2:23: error: expected \';\' before \'}\' token\n int main() { return 0 }\n                       ^\n" } 
Aug 31 19:01:33  
Aug 31 19:01:33 =========== If your build fails, please take a look at the log above for possible reasons =========== 
Aug 31 19:01:33 Compile requests                 0 
Aug 31 19:01:33 Compile requests executed        0 
Aug 31 19:01:33 Cache hits                       0 
Aug 31 19:01:33 Cache misses                     0 
Aug 31 19:01:33 Cache timeouts                   0 
Aug 31 19:01:33 Cache read errors                0 
Aug 31 19:01:33 Forced recaches                  0 
Aug 31 19:01:33 Cache write errors               0

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 or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 35 times.

[ghstack-poisoned]

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

zou3519

Still working my way through a list of all TensorIteratorConfig() callsites. I am around 30% done through that list. Here are some initial comments

aten/src/ATen/native/TensorIterator.h

aten/src/ATen/native/TensorShape.cpp

aten/src/ATen/native/UnaryOps.cpp

zou3519 · 2020-08-28T21:23:44Z

aten/src/ATen/native/TensorAdvancedIndexing.cpp

+  at::assert_no_internal_overlap(result);
+  at::assert_no_partial_overlap(result, self);
+  at::assert_no_partial_overlap(result, mask);
+


This file also adds memory overlap support for:

index_select out= variant (we should add a test for this)

masked_fill_ (we should add a test for this)

masked_select out=variant (we should add a test for this)

@peterbell10 did you want to add tests for these? I'm fine without them but it is nice to have them to check parity between CPU/CUDA (and to detect regressions)

They have tests in #43423 and I moved the asserts to that PR as well.

Got it, I didn't realize. Thanks for clarifying

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

zou3519 · 2020-08-31T14:44:07Z

pytorch/aten/src/ATen/native/FunctionOfAMatrixUtils.cpp

Line 86 in a860be8

.add_output(output_restrided)

I think we shouldn't check memory overlap here. It looks like the output is being restrided so that it has a zero stride, so that would trigger the memory overlap check always when this function is called.

aten/src/ATen/native/cuda/ScatterGatherKernel.cu

zou3519

OK, done reading through all callsites of TensorIteratorConfig. I had some comments above, other than that, this looks good

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

facebook-github-bot · 2020-09-02T16:14:02Z

@zou3519 merged this pull request in 5807bb9.

albanD · 2020-10-09T18:47:42Z

BC-breaking note:
Both PRs in this stack are BC-breaking because:

they fix silent correctness errors; something that used to be silently incorrect now errors out
Any external users of TensorIterator now always get the memory overlap check

TensorIteratorConfig: Check memory overlap by default

6b17799

[ghstack-poisoned]

pytorchbot added the open source label Aug 21, 2020

Update on "TensorIteratorConfig: Check memory overlap by default"

842c2ba

[ghstack-poisoned]

peterbell10 requested a review from zou3519 August 22, 2020 16:00

ngimel added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 24, 2020

peterbell10 added 4 commits August 25, 2020 14:54

Update on "TensorIteratorConfig: Check memory overlap by default"

5f548bf

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

Update on "TensorIteratorConfig: Check memory overlap by default"

e501e02

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

Update on "TensorIteratorConfig: Check memory overlap by default"

2752b72

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

Update on "TensorIteratorConfig: Check memory overlap by default"

3c83cf1

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

zou3519 reviewed Aug 28, 2020

View reviewed changes

peterbell10 added 2 commits August 29, 2020 17:15

Update on "TensorIteratorConfig: Check memory overlap by default"

7f87165

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

Update on "TensorIteratorConfig: Check memory overlap by default"

950f8c7

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

zou3519 reviewed Aug 31, 2020

View reviewed changes

aten/src/ATen/native/cuda/ScatterGatherKernel.cu Outdated Show resolved Hide resolved

zou3519 reviewed Aug 31, 2020

View reviewed changes

peterbell10 added 2 commits August 31, 2020 18:37

Update on "TensorIteratorConfig: Check memory overlap by default"

772b6f4

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

Update on "TensorIteratorConfig: Check memory overlap by default"

4ff43a9

Differential Revision: [D23298653](https://our.internmc.facebook.com/intern/diff/D23298653) [ghstack-poisoned]

zou3519 approved these changes Aug 31, 2020

View reviewed changes

facebook-github-bot closed this in 5807bb9 Sep 2, 2020

facebook-github-bot added the merged label Sep 2, 2020

facebook-github-bot deleted the gh/peterbell10/8/head branch September 6, 2020 14:16

albanD added the module: bc-breaking Related to a BC-breaking change label Oct 9, 2020

mruberry added the Merged label Oct 28, 2020

TensorIteratorConfig: Check memory overlap by default #43422

TensorIteratorConfig: Check memory overlap by default #43422

Uh oh!

Conversation

peterbell10 commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Aug 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 2 new failures recognized by patterns

pytorch_linux_xenial_py3_6_gcc5_4_ge_config_simple_test (1/2)

pytorch_linux_xenial_py3_6_gcc5_4_test (2/2)

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zou3519 Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

zou3519 Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

peterbell10 Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

zou3519 Aug 31, 2020

Choose a reason for hiding this comment

Uh oh!

zou3519 commented Aug 31, 2020

Uh oh!

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 2, 2020

Uh oh!

albanD commented Oct 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

peterbell10 commented Aug 21, 2020 •

edited

Loading

dr-ci bot commented Aug 21, 2020 •

edited

Loading