-
Notifications
You must be signed in to change notification settings - Fork 26.3k
C++ API: torch::nn::Softmax2d #27509
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks a lot for the great work! We will need some fixes as suggested in #27446. |
yf225
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the awesome work @nuka137! I left a very minor comment, and we should be able to merge it very soon :D
| /// about the exact behavior of this module. | ||
| class TORCH_API Softmax2dImpl : public torch::nn::Cloneable<Softmax2dImpl> { | ||
| public: | ||
| Softmax2dImpl(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the default constructor because it will be created by default :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it
|
|
||
| // ============================================================================ | ||
|
|
||
| Softmax2dImpl::Softmax2dImpl() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the default constructor here :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I fixed it.
|
Thanks for reviewing. |
yf225
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much for the awesome work @nuka137!
|
@pytorchbot rebase this please |
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Add torch::nn::Softmax2d module support for the C++ API. Softmax2d only supports module in Python API, so this PR adds only module support as well. This PR is WIP because it uses the function in pytorch#27446 . After pytorch#27446 is merged, I will remove WIP. Related Issue: pytorch#25883 Reviewer: yf225 Pull Request resolved: pytorch#27509 Differential Revision: D17899715 Pulled By: yf225 fbshipit-source-id: bd891bc995f5a92bf4f5405f8bf07d1bd5de2479
Add torch::nn::Softmax2d module support for the C++ API.
Softmax2d only supports module in Python API, so this PR adds only module support as well.
This PR is WIP because it uses the function in #27446 .
After #27446 is merged, I will remove WIP.
Related Issue: #25883
Reviewer: @yf225