TensorIterator: Check for memory overlap in all `nullary_op`s #43421

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

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
TensorIterator: Check for memory overlap in all nullary_ops #43421 TensorIterator: Check for memory overlap in all nullary_ops
TensorIterator: Check for memory overlap in all compare_ops #43420 TensorIterator: Check for memory overlap in all compare_ops
TensorIterator: Check for memory overlap in all binary_ops #43419 TensorIterator: Check for memory overlap in all binary_ops
TensorIterator: Check memory overlap in all unary_ops #43418 TensorIterator: Check memory overlap in all unary_ops

Differential Revision: D23298654

[ghstack-poisoned]

dr-ci · 2020-08-21T20:03:57Z

💊 CI failures summary and remediations

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

✅ None of the CI failures appear to be your fault 💚

1/1 broken upstream at merge base 01f974e on Aug 27 from 2:34pm to 6:12pm PDT (11 commits; d1c4d75 - 654ab20)

🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

Since your merge base is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

Check out the recency history of this "viable master" tracking branch.

pytorch_windows_vs2019_py36_cuda10.1_test2 on Aug 27 from 2:34pm to 6:12pm PDT (11 commits; d1c4d75 - 654ab20)
- 🔁 rerun

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 27 times.

…p`s" [ghstack-poisoned]

zou3519 · 2020-08-24T21:48:56Z

test/test_torch.py

+            ("log_normal_", ()),
+            ("exponential_", ()),
+            ("geometric_", (0.5,)),
+            ("normal_", ()),


We should add a test for bernoulli_, too, since we're turning on memory checking for it:

pytorch/aten/src/ATen/native/cuda/DistributionBernoulli.cu

Line 38 in 284ff04

auto iter = TensorIterator::nullary_op(self);

pytorch/aten/src/ATen/native/cpu/DistributionTemplates.h

Line 345 in 284ff04

auto iter = TensorIterator::nullary_op(self);

Some versions of bernoulli go through different code paths :/ we should probably try to keep them consistent in a single PR:

pytorch/aten/src/ATen/native/cpu/DistributionTemplates.h

Lines 318 to 322 in 284ff04

auto iter = TensorIteratorConfig()

.add_output(self)

.add_input(p)

.check_all_same_dtype(false)

.build();

(see #43421 (comment))

zou3519

I went through all use cases of nullary_op. Those look fine to me; we should add a test case for bernoulli_

zou3519 · 2020-08-24T21:54:21Z

Hmm, actually, I see the bernoulli_ test code in the next diff up. Is that because it is only in the next diff that we finish moving all of the bernoulli code paths to checking memory overlap? If so then I think it is OK to leave it there.

I am probably going to try to land #43418 - #43421 together after this stack gets rebased and I'll take a look at #43422 and #43423 tomorrow

zou3519

pre-emptive approve b/c the changes remaining are not that big

peterbell10 · 2020-08-25T13:10:29Z

Hmm, actually, I see the bernoulli_ test code in the next diff up. Is that because it is only in the next diff that we finish moving all of the bernoulli code paths to checking memory overlap?

There are different code paths depending on if the probability is a tensor. One goes through nullary_op and is checked in this PR, the other doesn't

pytorch/aten/src/ATen/native/cpu/DistributionTemplates.h

Line 318 in 284ff04

auto iter = TensorIteratorConfig()

zou3519 · 2020-08-25T13:51:08Z

There are different code paths depending on if the probability is a tensor. One goes through nullary_op and is checked in this PR, the other doesn't

Okay, I see. This is fine as-is then

…p`s" Differential Revision: [D23298654](https://our.internmc.facebook.com/intern/diff/D23298654) [ghstack-poisoned]

facebook-github-bot · 2020-08-28T18:14:51Z

@zou3519 merged this pull request in c177d25.

TensorIterator: Check for memory overlap in all nullary_ops

4f757ed

[ghstack-poisoned]

pytorchbot added the open source label Aug 21, 2020

Update on "TensorIterator: Check for memory overlap in all `nullary_o…

7e9954e

…p`s" [ghstack-poisoned]

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

zou3519 reviewed Aug 24, 2020

View reviewed changes

zou3519 approved these changes Aug 24, 2020

View reviewed changes

zou3519 approved these changes Aug 25, 2020

View reviewed changes

peterbell10 added 4 commits August 25, 2020 14:54

Update on "TensorIterator: Check for memory overlap in all `nullary_o…

b32bb23

…p`s" Differential Revision: [D23298654](https://our.internmc.facebook.com/intern/diff/D23298654) [ghstack-poisoned]

Update on "TensorIterator: Check for memory overlap in all `nullary_o…

dc4ea3b

…p`s" Differential Revision: [D23298654](https://our.internmc.facebook.com/intern/diff/D23298654) [ghstack-poisoned]

Update on "TensorIterator: Check for memory overlap in all `nullary_o…

be1fa1c

…p`s" Differential Revision: [D23298654](https://our.internmc.facebook.com/intern/diff/D23298654) [ghstack-poisoned]

Update on "TensorIterator: Check for memory overlap in all `nullary_o…

a356b80

…p`s" Differential Revision: [D23298654](https://our.internmc.facebook.com/intern/diff/D23298654) [ghstack-poisoned]

facebook-github-bot closed this in c177d25 Aug 28, 2020

facebook-github-bot added the merged label Aug 28, 2020

facebook-github-bot deleted the gh/peterbell10/7/head branch September 1, 2020 14:17

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TensorIterator: Check for memory overlap in all `nullary_op`s #43421

TensorIterator: Check for memory overlap in all `nullary_op`s #43421

Uh oh!

peterbell10 commented Aug 21, 2020 •

edited by zou3519

Loading

Uh oh!

dr-ci bot commented Aug 21, 2020 •

edited

Loading

Uh oh!

zou3519 Aug 24, 2020 •

edited

Loading

Uh oh!

zou3519 Aug 24, 2020

Uh oh!

zou3519 left a comment

Uh oh!

zou3519 commented Aug 24, 2020

Uh oh!

zou3519 left a comment

Uh oh!

peterbell10 commented Aug 25, 2020 •

edited

Loading

Uh oh!

zou3519 commented Aug 25, 2020

Uh oh!

facebook-github-bot commented Aug 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	auto iter = TensorIteratorConfig()
	.add_output(self)
	.add_input(p)
	.check_all_same_dtype(false)
	.build();

TensorIterator: Check for memory overlap in all nullary_ops #43421

TensorIterator: Check for memory overlap in all nullary_ops #43421

Uh oh!

Conversation

peterbell10 commented Aug 21, 2020 • edited by zou3519 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

🚧 1 fixed upstream failure:

Uh oh!

zou3519 Aug 24, 2020 • 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!

zou3519 Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

zou3519 commented Aug 24, 2020

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

zou3519 commented Aug 25, 2020

Uh oh!

facebook-github-bot commented Aug 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

TensorIterator: Check for memory overlap in all `nullary_op`s #43421

TensorIterator: Check for memory overlap in all `nullary_op`s #43421

peterbell10 commented Aug 21, 2020 •

edited by zou3519

Loading

dr-ci bot commented Aug 21, 2020 •

edited

Loading

zou3519 Aug 24, 2020 •

edited

Loading

peterbell10 commented Aug 25, 2020 •

edited

Loading