Skip to content

Refactor ForeachUnaryOp.cu#49248

Closed
izdeby wants to merge 23 commits intogh/izdeby/70/basefrom
gh/izdeby/70/head
Closed

Refactor ForeachUnaryOp.cu#49248
izdeby wants to merge 23 commits intogh/izdeby/70/basefrom
gh/izdeby/70/head

Conversation

@izdeby
Copy link
Copy Markdown
Contributor

@izdeby izdeby commented Dec 11, 2020

Stack from ghstack:

Differential Revision: D25502940


  • Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

[ghstack-poisoned]
Iurii Zdebskyi added 4 commits December 11, 2020 12:31
Differential Revision: [D25502940](https://our.internmc.facebook.com/intern/diff/D25502940)

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
- Add `has_bool_tensor` checks that will be crucial in next PRs in this stack

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
- Add `has_bool_tensor` checks that will be crucial in next PRs in this stack

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
- Add `has_bool_tensor` checks that will be crucial in next PRs in this stack

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
- Add `has_bool_tensor` checks that will be crucial in next PRs in this stack

[ghstack-poisoned]
@izdeby izdeby changed the title Refactor ForeachUnaryOps.cu Refactor ForeachUnaryOp.cu Dec 15, 2020
Iurii Zdebskyi added 2 commits December 15, 2020 16:13
Differential Revision: [D25502940](https://our.internmc.facebook.com/intern/diff/D25502940)

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
- Add `has_bool_tensor` checks that will be crucial in next PRs in this stack

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

[ghstack-poisoned]
Copy link
Copy Markdown
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

Have some questions and suggestions

Comment on lines +341 to +351
std::vector<Tensor> foreach_tensor_neg_cuda(TensorList tensors) {
check_foreach_api_restrictions(tensors);
TORCH_CHECK(tensors[0].scalar_type() != kBool,
"Negation, the `-` operator, on a bool tensor is not supported. "
"If you are trying to invert a mask, use the `~` or `logical_not()` operator instead.");

if (!can_use_fast_route(tensors)) {
return at::native::foreach_tensor_neg_slow(tensors);
}

return foreach_unary_op_complex_bfloat16<std::negate>(tensors);
return all_types_half_bfloat16<std::negate>(tensors);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is the new check needed? Also, this updates the semantics of neg, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It doesnt change the behavior but an error. it will match the error of torch.neg in case of bool input

Comment on lines +404 to +406
if (at::isComplexType(t.scalar_type()) ||
at::isIntegralType(t.scalar_type(), /*includeBool=*/true)) {
has_complex_or_integer = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fixing a bug, right? Should we add a test for it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It came from future PRs. I was breaking one PR into pieces and it came here by mistake. removed for now.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

[ghstack-poisoned]
@izdeby izdeby requested a review from zou3519 December 17, 2020 20:57
Copy link
Copy Markdown
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

Thanks, everything is clearer now!

Iurii Zdebskyi added 5 commits December 17, 2020 15:52
Differential Revision: [D25502940](https://our.internmc.facebook.com/intern/diff/D25502940)

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

[ghstack-poisoned]
@izdeby izdeby mentioned this pull request Jan 26, 2021
Iurii Zdebskyi added 2 commits January 26, 2021 14:03
Differential Revision: [D25502940](https://our.internmc.facebook.com/intern/diff/D25502940)

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

[ghstack-poisoned]
Iurii Zdebskyi added 4 commits January 27, 2021 07:18
Differential Revision: [D25502940](https://our.internmc.facebook.com/intern/diff/D25502940)

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

----------
- Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.

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

@izdeby merged this pull request in 5cf3278.

@zou3519
Copy link
Copy Markdown
Contributor

zou3519 commented Feb 3, 2021

@izdeby were Natalia's comments above addressed? #49248 (comment) and #49248 (comment)

@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been reverted by d1bc1ab.

@facebook-github-bot facebook-github-bot deleted the gh/izdeby/70/head branch February 6, 2021 15:15
izdeby pushed a commit that referenced this pull request Feb 18, 2021
original PR - #49248

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

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Feb 18, 2021
original PR - #49248

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

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Feb 22, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Mar 1, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Mar 2, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Mar 3, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Mar 4, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
izdeby pushed a commit that referenced this pull request Mar 4, 2021
original PR - #49248

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

-------
Refactor ForeachUnaryOps.cu to better see what dtypes are supported for ops.
Update abs test to test ints.

[ghstack-poisoned]
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.

4 participants