Skip to content

【2.0 API】Enhance affine grid operator#26385

Merged
wanghaoshuang merged 7 commits intoPaddlePaddle:developfrom
wanghaoshuang:grid_api
Aug 25, 2020
Merged

【2.0 API】Enhance affine grid operator#26385
wanghaoshuang merged 7 commits intoPaddlePaddle:developfrom
wanghaoshuang:grid_api

Conversation

@wanghaoshuang
Copy link
Contributor

@wanghaoshuang wanghaoshuang commented Aug 18, 2020

PR types

Others

PR changes

OPs

Describe

  1. Add cuda kernel
  2. Add align corners options

@wanghaoshuang
Enhance affine grid operator:
a949914 
1. Add cuda kernel
2. Add align corners options
test=develop
@paddle-bot-old
Copy link

paddle-bot-old bot commented Aug 18, 2020

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@wanghaoshuang
Copy link
Contributor Author

wanghaoshuang commented Aug 18, 2020 •
edited
Loading

问题一、能否将对3D的支持推迟一下？

  1. 时间紧迫，支持3D引入的未知风险，有可能导致该PR延期合入Paddle2.0
  2. 支持3D的需求相对不是很多，cudnn的实现只支持2D

问题二、还需要为paddle.nn.functional.affine_grid写单测么？

  1. paddle.nn.functional.affine_grid是直接alias自paddle.fluid.layers.affine_grid, 也就是完全一样的实现

willthefrog
willthefrog requested changes Aug 18, 2020
View reviewed changes
@willthefrog
Copy link
Contributor

willthefrog commented Aug 18, 2020

问题一、能否将对3D的支持推迟一下？

  1. 时间紧迫，支持3D引入的未知风险，有可能导致该PR延期合入Paddle2.0
  2. 支持3D的需求相对不是很多，cudnn的实现只支持2D

问题二、还需要为paddle.nn.functional.affine_grid写单测么？

  1. paddle.nn.functional.affine_grid是直接alias自paddle.fluid.layers.affine_grid, 也就是完全一样的实现
  1. yes, only add 3d support if time permits
  2. a new version should be add in nn.functional, as there are changes to the API.

willthefrog
willthefrog requested changes Aug 19, 2020
View reviewed changes
paddle/fluid/operators/affine_grid_op.cu Outdated
namespace ops = paddle::operators;
REGISTER_OP_CUDA_KERNEL(
affine_grid,
ops::AffineGridOpKernel<paddle::platform::CUDADeviceContext, float>,
Copy link
Contributor

@willthefrog willthefrog Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the cuda version mostly reused the eigen implementation? some preliminary benchmark would be nice if time permits.

Copy link
Contributor Author

@wanghaoshuang wanghaoshuang Aug 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added CUDA kernel.
I will give some performance benchmark on CUDNN version and CUDA kernel version in the future.
Currently, it will use CUDNN kernel when align_corners is true and use CUDA kernel when align_corners is false.

willthefrog
willthefrog previously approved these changes Aug 24, 2020
View reviewed changes
Copy link
Contributor

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need subsequent PR to add support for higher dimensions, otherwise LGTM

saxon-zh
saxon-zh previously approved these changes Aug 24, 2020
View reviewed changes
Copy link

@saxon-zh saxon-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wanghaoshuang wanghaoshuang dismissed stale reviews from saxon-zh and willthefrog via de31410 August 24, 2020 07:03
willthefrog
willthefrog approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@willthefrog willthefrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wanghaoshuang wanghaoshuang merged commit a065a24 into PaddlePaddle:develop Aug 25, 2020
@wanghaoshuang wanghaoshuang deleted the grid_api branch August 25, 2020 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willthefrog willthefrog willthefrog approved these changes

@saxon-zh saxon-zh saxon-zh left review comments

@qingqing01 qingqing01 Awaiting requested review from qingqing01

@heavengate heavengate Awaiting requested review from heavengate

@XiaoguangHu01 XiaoguangHu01 Awaiting requested review from XiaoguangHu01

@LielinJiang LielinJiang Awaiting requested review from LielinJiang

@jzhang533 jzhang533 Awaiting requested review from jzhang533

@swtkiwi swtkiwi Awaiting requested review from swtkiwi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wanghaoshuang @willthefrog @saxon-zh