Implement named inference rule for mul #23193
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Featuring: - The named inference rule is actually more complicated than it should be. This is because I'd like to "swap out" named tensor rules (I'm experimenting with two of them) easily. In the future when things are more stable I will return to the callsites and clean up the API. Test Plan: - [namedtensor ci]
pytorchbot
added
module: internals
This was referenced Jul 22, 2019
Implement named inference rule for mul Featuring: - The named inference rule is actually more complicated than it should be. This is because I'd like to "swap out" named tensor rules (I'm experimenting with two of them) easily. In the future when things are more stable I will return to the callsites and clean up the API. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
Implement named inference rule for mul Featuring: - The named inference rule is actually more complicated than it should be. This is because I'd like to "swap out" named tensor rules (I'm experimenting with two of them) easily. In the future when things are more stable I will return to the callsites and clean up the API. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
Implement named inference rule for mul Featuring: - The named inference rule is actually more complicated than it should be. This is because I'd like to "swap out" named tensor rules (I'm experimenting with two of them) easily. In the future when things are more stable I will return to the callsites and clean up the API. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
Implement named inference rule for mul Featuring: - The named inference rule is actually more complicated than it should be. This is because I'd like to "swap out" named tensor rules (I'm experimenting with two of them) easily. In the future when things are more stable I will return to the callsites and clean up the API. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
colesbury
reviewed
Jul 23, 2019
aten/src/ATen/native/BinaryOps.cpp
Outdated
| @@ -80,6 +83,17 @@ Tensor& mul_out(Tensor& result, const Tensor& self, const Tensor& other) { | |||
| return at::_sparse_mul_out(result, self, other); | |||
| } | |||
| at::assert_no_internal_overlap(result, "mul"); | |||
| #ifdef BUILD_NAMEDTENSOR | |||
There was a problem hiding this comment.
Can you implement this in one place in TensorIterator instead of each of the call sites?
There was a problem hiding this comment.
Sure, let me look into it.
There was a problem hiding this comment.
Thanks for the great suggestion, @colesbury, it worked out really well
Implement named inference rule for mul Featuring: - Implemented name propagation directly in TensorIterator. This also probably enables a lot of other functions but I'll add/enable them in another PR. - Cleaned up old callsites that were using TensorIterator since TensorIterator automatically propagates names. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
Implement named inference rule for mul Featuring: - Implemented name propagation directly in TensorIterator. This also probably enables a lot of other functions but I'll add/enable them in another PR. - Cleaned up old callsites that were using TensorIterator since TensorIterator automatically propagates names. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
This was referenced Jul 23, 2019
Implement named inference rule for mul Featuring: - Implemented name propagation directly in TensorIterator. This also probably enables a lot of other functions but I'll add/enable them in another PR. - Cleaned up old callsites that were using TensorIterator since TensorIterator automatically propagates names. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
nairbv
reviewed
Jul 23, 2019
|
|
||
| // propagate names | ||
| for (int i = 0; i < num_outputs_; i++) { | ||
| auto& op = operands_[i]; |
There was a problem hiding this comment.
Do you happen to know why Tensor output(int arg=0) doesn't return a Tensor& ? Seems like it would be nice to be able to use that here.
nairbv
approved these changes
Jul 23, 2019
This was referenced Jul 24, 2019
Implement named inference rule for mul Featuring: - Implemented name propagation directly in TensorIterator. This also probably enables a lot of other functions but I'll add/enable them in another PR. - Cleaned up old callsites that were using TensorIterator since TensorIterator automatically propagates names. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
nairbv
approved these changes
Jul 25, 2019
Implement named inference rule for mul Featuring: - Implemented name propagation directly in TensorIterator. This also probably enables a lot of other functions but I'll add/enable them in another PR. - Cleaned up old callsites that were using TensorIterator since TensorIterator automatically propagates names. Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
This was referenced Jul 26, 2019
Merged
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Jul 29, 2019
Summary: Pull Request resolved: pytorch/pytorch#23193 Test Plan: - [namedtensor ci] gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head Imported from OSS Differential Revision: D16494401 Pulled By: zou3519 fbshipit-source-id: 0e2395d7de39158ec51feed5da0389715ec52600
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Featuring:
probably enables a lot of other functions but I'll add/enable them in
another PR.
TensorIterator automatically propagates names.
Test Plan:
gh-metadata: pytorch pytorch 23193 gh/zou3519/75/head
Differential Revision: D16494401