interpolate#14123
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Is there a reason why this isn't written as a native function?
There was a problem hiding this comment.
If it were a native function the function would have to have 4 different variants, one for each combination of size & scale_factor. Plus doing it this way makes the logic in c++ and the existing python function 1-1.
It is kind of annoying case. It might be better to move it to builtin_functions.cpp, but it would require having 4 variants and/or adding some overhead to support them, plus there are other features that are not yet implemented in builtin_functions that this would require.
c3a07b1 to
8b59a67
Compare
There was a problem hiding this comment.
nn_module_tests isn't used anywhere anymore, so these could all be deleted
driazati
left a comment
There was a problem hiding this comment.
Looks good for now to unblock interpolate, we should follow up for a better solution
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
f8025af to
34016f4
Compare
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@eellison is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
|
||
| @weak_script_method | ||
| def forward(self, input): | ||
| warnings.warn("nn.{} is deprecated. Use nn.functional.interpolate instead.".format(self.name)) |
Summary: Add support for interpolate and upsampling in weak_script mode. Because the function parameters are overloaded, i had to add it as a builtin op. For interpolate: size can be ?int | int[]?, and scale_factor can be ?float | float[]?. Every combination of the two parameters needs to be supported. The same logic applies for upsample_nearest, upsample_bilinear, and upsample. There are a few fixes that I came to along the way. Pull Request resolved: pytorch#14123 Differential Revision: D13278923 Pulled By: eellison fbshipit-source-id: e59729034369be4ce4b747291a3d1c74e135b869
Summary: IIRC we decided to remove warning in code in pytorch#11568. This got reverted accidentally in pytorch#14123. Pull Request resolved: pytorch#17921 Differential Revision: D14422811 Pulled By: ailzhang fbshipit-source-id: 7067264bd1d3e3b7861d29e18ade2969ed705ca1
Add support for interpolate and upsampling in weak_script mode.
Because the function parameters are overloaded, i had to add it as a builtin op. For interpolate:
size can be ?int | int[]?, and scale_factor can be ?float | float[]?. Every combination of the two parameters needs to be supported.
The same logic applies for upsample_nearest, upsample_bilinear, and upsample.
There are a few fixes that I came to along the way.