Implement nn.functional.interpolate based on upsample. by ailzhang · Pull Request #8591 · pytorch/pytorch

ailzhang · 2018-06-18T00:06:54Z

This PR addresses #5823.

fix docstring: upsample doesn't support LongTensor
Enable float scale up & down sampling for linear/bilinear/trilinear modes. (following @ssnl 's commit)
Enable float scale up & down sampling for nearest mode. Note that our implementation is slightly different from TF that there's actually no "align_corners" concept in this mode.
Add a new interpolate function API to replace upsample. Add deprecate warning for upsample.
Add an area mode which is essentially Adaptive_average_pooling into resize_image.
Add test cases for interpolate in test_nn.py
Add a few comments to help understand *linear interpolation code.
There is only "*cubic" mode missing in resize_images API which is pretty useful in practice. And it's labeled as hackamonth here [Feature Request] Unstructured Interpolation for PyTorch Tensor #1552. I discussed with @ssnl that we probably want to implement all new ops in ATen instead of THNN/THCUNN. Depending on the priority, I could either put it in my queue or leave it for a HAMer.
After the change, the files named as Upsampling.c works for both up/down sampling. I could rename the files if needed.

vadimkantorov · 2018-06-18T09:23:01Z

does it enable bilinear interpolation for ByteTensors as well? (#5580)

ssnl

Thanks for doing this. The kernel code mostly looks fine, but I didn't spend too much time on it. Here are some general comments:

The kernel names are wrong. The NN kernels shouldn't have interp. All kernels should have correct dimension number.
For the special copying case, can we use THTensor_(copy) instead of a nested loop?

aten/src/THCUNN/generic/TemporalUpSamplingNearest.cu

aten/src/THNN/generic/SpatialUpSamplingNearest.c

aten/src/THNN/generic/linear_upsampling.h

test/test_nn.py

aten/src/THCUNN/SpatialUpSamplingNearest.cu

ailzhang · 2018-06-19T18:08:57Z

@vadimkantorov Actually all methods like pooling/interpolation don't support byte/half/long tensor. I would prefer to keep it in a separate diff, or even do it along with TH->Aten refactor. cc: @soumith @ssnl @fmassa What do you think?

ssnl · 2018-06-19T18:32:14Z

Yeah I think you should keep those out of this diff.

fmassa

Hi,

I've had a quick look and I think there are a few memory leaks that should be fixed.

Also, given that we support 1d, 2d and 3d data, I wonder if we should name it resize_images, or maybe something a bit more generic?

Not for this PR

For the future, we might want to specify which dimensions we want to interpolate, so that this is not specific to a given data format.

For example

a = torch.rand(1, 3, 100, 100)
A = torch.nn.functional.interpolate(a, axis=[-2. -1], mode='nearest')

# NHWC format
a = torch.rand(1, 100, 100, 3)
A = torch.nn.functional.interpolate(a, axis=[1, 2])

# non-batch mode, for single image
a = torch.rand(3, 100, 100)
A = torch.nn.functional.interpolate(a, axis=[-2, -1])

aten/src/THNN/generic/SpatialUpSamplingNearest.c

aten/src/THCUNN/SpatialUpSamplingNearest.cu

aten/src/THNN/generic/SpatialUpSamplingNearest.c

aten/src/THNN/generic/TemporalUpSamplingNearest.c

aten/src/THNN/generic/VolumetricUpSamplingNearest.c

vadimkantorov · 2018-06-22T10:53:59Z

Anothe naming suggestion is resample #4277 but maybe it’s a little less discoverable than interpolate

ailzhang · 2018-06-22T22:43:55Z

@pytorchbot retest this please

fmassa

I think this looks good to merge, thanks Ailing!

I have a bunch of minor nits that are not merge blockers, so feel free to postpone addressing them in a follow-up PR.

aten/src/THCUNN/generic/SpatialUpSamplingNearest.cu

aten/src/THCUNN/generic/THCUNN.h

aten/src/THCUNN/generic/TemporalUpSamplingLinear.cu

aten/src/THCUNN/generic/TemporalUpSamplingNearest.cu

aten/src/THNN/generic/THNN.h

aten/src/THNN/generic/TemporalUpSamplingNearest.c

aten/src/THNN/generic/VolumetricUpSamplingNearest.c

test/test_nn.py

torch/nn/modules/upsampling.py

fmassa · 2018-07-02T10:01:08Z

I have the impression that the ROCM build failure is unrelated https://ci.pytorch.org/jenkins/job/pytorch-builds/job/py2-clang3.8-rocmnightly-ubuntu16.04-build/1826//console , but might be better for others to have a look as well

fmassa · 2018-07-02T10:03:29Z

@pytorchbot retest this please

(let's see if it works with me :-) )

ailzhang · 2018-07-02T17:42:47Z

@pytorchbot retest this please

ailzhang · 2018-07-02T18:44:53Z

@pytorchbot retest this please

ailzhang · 2018-07-02T22:45:32Z

@pytorchbot retest this please

facebook-github-bot

@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

…ample

facebook-github-bot

@ailzhang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: This PR addresses #5823. * fix docstring: upsample doesn't support LongTensor * Enable float scale up & down sampling for linear/bilinear/trilinear modes. (following SsnL 's commit) * Enable float scale up & down sampling for nearest mode. Note that our implementation is slightly different from TF that there's actually no "align_corners" concept in this mode. * Add a new interpolate function API to replace upsample. Add deprecate warning for upsample. * Add an area mode which is essentially Adaptive_average_pooling into resize_image. * Add test cases for interpolate in test_nn.py * Add a few comments to help understand *linear interpolation code. * There is only "*cubic" mode missing in resize_images API which is pretty useful in practice. And it's labeled as hackamonth here #1552. I discussed with SsnL that we probably want to implement all new ops in ATen instead of THNN/THCUNN. Depending on the priority, I could either put it in my queue or leave it for a HAMer. * After the change, the files named as *Upsampling*.c works for both up/down sampling. I could rename the files if needed. Differential Revision: D8729635 Pulled By: ailzhang fbshipit-source-id: a98dc5e1f587fce17606b5764db695366a6bb56b

ailzhang · 2018-07-09T17:45:05Z

This is merged by facebook-github-bot but somehow didn't close automatically. Closing

Summary: This PR addresses pytorch#5823. * fix docstring: upsample doesn't support LongTensor * Enable float scale up & down sampling for linear/bilinear/trilinear modes. (following SsnL 's commit) * Enable float scale up & down sampling for nearest mode. Note that our implementation is slightly different from TF that there's actually no "align_corners" concept in this mode. * Add a new interpolate function API to replace upsample. Add deprecate warning for upsample. * Add an area mode which is essentially Adaptive_average_pooling into resize_image. * Add test cases for interpolate in test_nn.py * Add a few comments to help understand *linear interpolation code. * There is only "*cubic" mode missing in resize_images API which is pretty useful in practice. And it's labeled as hackamonth here pytorch#1552. I discussed with SsnL that we probably want to implement all new ops in ATen instead of THNN/THCUNN. Depending on the priority, I could either put it in my queue or leave it for a HAMer. * After the change, the files named as *Upsampling*.c works for both up/down sampling. I could rename the files if needed. Differential Revision: D8729635 Pulled By: ailzhang fbshipit-source-id: a98dc5e1f587fce17606b5764db695366a6bb56b

ailzhang requested review from apaszke, colesbury, ezyang, gchanan, soumith and zdevito as code owners June 18, 2018 00:06

ssnl reviewed Jun 19, 2018

View reviewed changes

ailzhang force-pushed the linear_sampling_float_scale branch 2 times, most recently from f216d18 to d0f1935 Compare June 21, 2018 04:26

fmassa requested changes Jun 22, 2018

View reviewed changes

ailzhang requested a review from zou3519 as a code owner June 22, 2018 19:47

ailzhang changed the title ~~Implement resize_images based on upsample.~~ Implement nn.functional.interpolate based on upsample. Jun 22, 2018

ssnl self-assigned this Jun 25, 2018

fmassa reviewed Jul 1, 2018

View reviewed changes

fmassa approved these changes Jul 2, 2018

View reviewed changes

facebook-github-bot reviewed Jul 3, 2018

View reviewed changes

soumith approved these changes Jul 3, 2018

View reviewed changes

Ailing added 2 commits July 5, 2018 23:01

fix doc: it doesn't support LongTensor, converting to float

c72e9de

float scale up/downsampling for *linear methods

6c61ef5

Ailing added 22 commits July 5, 2018 23:01

rename to match master branch

d1721f0

update test_nn

3162f9c

add comments to help understand *linear sampling code

9ac1e7a

Temporal case for nearest neighbor following TF implematation

a83b631

remove align_corners in nearest mode

7ce4c3e

clean up old code

6af31cc

Spatial case for nearest neighbor

064a796

Volumetric case for nearest neighbor

70d3b4b

Add a new resize_images module/function and deprecate warning for ups…

3958a11

…ample

Add area mode(adaptive_avg_pooling) to resize_images

70a52b6

add testcases for resize_images

f65b401

fix segfault in cuda backward. fix bug in 3d case; format code

9c4c2bf

rename to ResizeImages;rename kernels

5d1f218

gradInput is already contiguous; remove in *linear/nearest methods

288f25b

fix py2 tests

d03a5cd

fix legacy test with new c-python api

4b91f0f

update docs

a19c65b

fix test_utils

e4af62c

free tensor in special case

aa7c392

rename to interpolate; remove nn module; update docs

38d0085

fix indent; remove unnecessary header

5a9793a

input doesn't have to be contiguous in forward pass

f99c543

ailzhang force-pushed the linear_sampling_float_scale branch from e43fe9e to f99c543 Compare July 6, 2018 07:23

facebook-github-bot reviewed Jul 6, 2018

View reviewed changes

ailzhang closed this Jul 9, 2018

Conversation

ailzhang commented Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vadimkantorov commented Jun 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ssnl left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

ailzhang commented Jun 19, 2018

Uh oh!

ssnl commented Jun 19, 2018

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Not for this PR

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

vadimkantorov commented Jun 22, 2018

Uh oh!

ailzhang commented Jun 22, 2018

Uh oh!

fmassa left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

ailzhang commented Jun 18, 2018 •

edited

Loading

vadimkantorov commented Jun 18, 2018 •

edited

Loading