Skip to content

Support NHWC upsampling for CUDA. #35203

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

Closed
r-zenine opened this issue Mar 23, 2020 · 6 comments
Closed

Support NHWC upsampling for CUDA. #35203

r-zenine opened this issue Mar 23, 2020 · 6 comments
Labels
module: cuda Related to torch.cuda, and CUDA support in general module: memory format Memory format/layout related issues/changes (channels_last, nhwc) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@r-zenine
Copy link
Contributor

r-zenine commented Mar 23, 2020

🚀 Feature

Add support for NHWC images for upsampling nearest in CUDA.

pytorch/vision#1179 (comment)

Motivation

We need this work in order to match the work from This PR and have feature parity with CPU.

In addition, this would help us further support an ongoing work done with @fmassa to have efficient native image reading ops in torchvision.

Pitch

I propose the replicate work done in This PR](#34597) and add a new code path as it was done here:

https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/cpu/UpSampleKernel.cpp#L342-L351

This approach is simple. We would also stay consistent with the approach already taken. However, it's going to be hard to avoid unnecessary code duplication. I'll do my best.

@fmassa , @VitalyFedyunin Thoughts?

cc @ngimel @VitalyFedyunin @jamesr66a

@pbelevich pbelevich added module: cuda Related to torch.cuda, and CUDA support in general module: memory format Memory format/layout related issues/changes (channels_last, nhwc) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Mar 23, 2020
@r-zenine
Copy link
Contributor Author

Hi,
Small Update :
I see an opportunity to reuse most of the code if we manage to make this portion of the code parametrized by the memory layout.
https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/cuda/UpSampleNearest2d.cu#L46-L49

We could add a template parameter to the upsample_nearest2d_out_frame for memory layout and get the code snippet above into it a template function. We could then add an if statement somewhere around here :
https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/cuda/UpSampleNearest2d.cu#L195-L206

Thoughts ?

@VitalyFedyunin
Copy link
Contributor

Parameterized looks acceptable, but please avoid parameterizing on at::MemoryFormat value, as it might change in future. I think best would be parameterising on something like bool is_channels_last

@VitalyFedyunin
Copy link
Contributor

Nit: You might find this information useful https://github.com/pytorch/pytorch/wiki/Writing-memory-format-aware-operators

@r-zenine
Copy link
Contributor Author

r-zenine commented Apr 2, 2020

Hi @VitalyFedyunin.

Thanks for the resources, I will take a look and work on the changes.
I'll get back to you when I have a first PR ready for review.

@r-zenine
Copy link
Contributor Author

r-zenine commented Apr 6, 2020

Hi @VitalyFedyunin,
Thank you for the resources. I learned things :)

I have a question: I can see that for the cpu version, the parametrization is done on at::MemoryFormat, Would adding a is_channels_last bolean parameter introduce inconsistencies in the code ?

Do we add the parameter and update both CPU and GPU code ?
What are your thoughts about this ?

Thanks

@r-zenine
Copy link
Contributor Author

Hi,

I cannot manage to put time aside to work on the issue for the moment. My schedule got crazy.
Sorry for the inconvenience.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: cuda Related to torch.cuda, and CUDA support in general module: memory format Memory format/layout related issues/changes (channels_last, nhwc) triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

3 participants