Port avg_pool3d() to ATen #21732
Closed
Port avg_pool3d() to ATen #21732
Conversation
This file contains hidden or 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
pytorchbot
added
module: build
skrah
added
open source
module: porting
pytorchbot
added
module: build
skrah
removed
module: build
Contributor
Author
|
@ezyang I think this one is ready now. I've kept the existing frame functions but did not create new ones. The existing Also, |
Contributor
Sounds like an omission; nice catch :) |
ezyang
reviewed
Jun 14, 2019
| TORCH_INTERNAL_ASSERT(kernel_size.size() == 3 && | ||
| (stride.empty() || stride.size() == 3) && | ||
| (padding.size() == 1 || padding.size() == 3), | ||
| "avg_pool3d: all IntArrayRef sizes must be 3"); |
Contributor
There was a problem hiding this comment.
Huh, do we actually check this from the C++ frontend? If not, this probably should be a CHECK.
Contributor
Author
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Contributor
|
This couldn't be landed due to internal issues. I am taking a look. |
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Jun 18, 2019
Summary: This will need a conflict resolution once avg_pool2d() has been merged. Pull Request resolved: pytorch/pytorch#21732 Differential Revision: D15824923 Pulled By: ezyang fbshipit-source-id: 83341e0209b660aecf788272079d8135d78b6ff1
Contributor
Contributor
|
Oof, we should really fix this in the C++ frontend somehow. I guess we
can bandaid it for now by adding logic to replicate in each operator
individually...
Excerpts from Stefan Krah's message of 2019-06-21 11:54:45 -0700:
… skrah commented on this pull request.
> +
+void avg_pool3d_out_cpu_template(
+ Tensor& output,
+ const Tensor& input_,
+ IntArrayRef kernel_size,
+ IntArrayRef stride,
+ IntArrayRef padding,
+ bool ceil_mode,
+ bool count_include_pad)
+{
+ // #20866 [JIT] stride.empty() is passed through
+ // #20866 [LIBTORCH] IntegrationTest.MNIST: padding.size() == 1
+ TORCH_INTERNAL_ASSERT(kernel_size.size() == 3 &&
+ (stride.empty() || stride.size() == 3) &&
+ (padding.size() == 1 || padding.size() == 3),
+ "avg_pool3d: all IntArrayRef sizes must be 3");
@ezyang Good question, it actually came up now in #22032, which showed that people are using the C++ frontend like the Python functions (expecting that single integers are repeated automatically).
So it's a CHECK now (#22075).
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
This will need a conflict resolution once avg_pool2d() has been merged.