C++ API: torch::nn::LPPool1d #27800
C++ API: torch::nn::LPPool1d #27800
Conversation
| }; | ||
|
|
||
| /// `LPPoolOptions` specialized for 1-D lppool. | ||
| using LPPool1dOptions = LPPoolOptions<1>; |
There was a problem hiding this comment.
I believe we should add template struct LPPoolOptions<1>; in torch/csrc/api/src/nn/options/pooling.cpp, similar to the other pooling options.
There was a problem hiding this comment.
Added template struct LPPoolOptions<1> to torch/csrc/api/src/nn/options/pooling.cpp.
| template <size_t D, typename Derived> | ||
| void LPPoolImpl<D, Derived>::pretty_print(std::ostream& stream) const { | ||
| stream << std::boolalpha | ||
| << "torch::nn::LPPool" << D << "d(" |
There was a problem hiding this comment.
It would be awesome to print norm_type as well, to match the Python version even better :D
There was a problem hiding this comment.
Added norm_type to print.
| avg_pool_options.stride(stride); | ||
| out = avg_pool1d(input.pow(norm_type), avg_pool_options); | ||
|
|
||
| return ((torch::sign(out) * torch::relu(torch::abs(out))) * (*kernel_size)[0]).pow(1. / norm_type); |
There was a problem hiding this comment.
I feel that it might be easier to maintain Python version parity if we write this function in the following way:
Tensor out = avg_pool1d(
input.pow(options.norm_type()),
AvgPool1dOptions(options.kernel_size()).stride(options.stride()).padding(0).ceil_mode(options.ceil_mode()));
return (torch::sign(out) * relu(torch::abs(out))).mul((*kernel_size)[0]).pow(1. / norm_type);| template <size_t D> | ||
| struct LPPoolOptions { | ||
| LPPoolOptions(ExpandingArray<D> kernel_size) | ||
| : kernel_size_(kernel_size), stride_(kernel_size) {} |
There was a problem hiding this comment.
I believe norm_type is a non-optional argument for the LPPool1d constructor in Python version, and we should take it in LPPoolOptions's constructor as well.
There was a problem hiding this comment.
OK. I added norm_type as a non-optional argument.
| LPPoolOptions(ExpandingArray<D> kernel_size) | ||
| : kernel_size_(kernel_size), stride_(kernel_size) {} | ||
|
|
||
| TORCH_ARG(float, norm_type) = 1.0f; |
There was a problem hiding this comment.
I believe norm_type doesn't have a default value in Python version
There was a problem hiding this comment.
I removed default value.
|
@pytorchbot rebase this please |
|
Thank you. |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Add torch::nn::LPPool1d module and functional support for the C++ API. Related Issue: pytorch#25883 Reviewer: yf225 Pull Request resolved: pytorch#27800 Differential Revision: D18045040 Pulled By: yf225 fbshipit-source-id: e61fefe9efec3423f7a93dd1e946f3e380122927
5 participants
Add torch::nn::LPPool1d module and functional support for the C++ API.
Related Issue: #25883
Reviewer: @yf225