Skip to content

Add roi pool #35084

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Sep 27, 2021
Merged

Add roi pool #35084

merged 6 commits into from
Sep 27, 2021

Conversation

@lyuwenyu
Copy link
Contributor

@lyuwenyu lyuwenyu commented Aug 23, 2021 •
edited
Loading

PR types

New features

PR changes

OPs

Describe

@paddle-bot-old
Copy link

paddle-bot-old bot commented Aug 23, 2021

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

@lyuwenyu lyuwenyu force-pushed the roi_pool_L branch 2 times, most recently from 84aaf42 to ea0b5c4 Compare August 23, 2021 11:19
@lyuwenyu lyuwenyu changed the title add roi pool Add roi pool Aug 24, 2021
@lyuwenyu lyuwenyu mentioned this pull request Aug 24, 2021
Closed
qingqing01
qingqing01 reviewed Sep 16, 2021
View reviewed changes
python/paddle/tests/test_ops_roi_pool.py Outdated
Copy link
Contributor

@qingqing01 qingqing01 Sep 16, 2021

Choose a reason for hiding this comment

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

结论正确性，可以 复用

Paddle/python/paddle/fluid/tests/unittests/test_roi_pool_op.py

Line 60 in cee7043

def calc_roi_pool(self):
这个文件里的函数，抽取公共函数，进行测试。

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 16, 2021

Choose a reason for hiding this comment

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

好的 正确性在test_value里 有验证具体计算的结果

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@qingqing01 qingqing01 Sep 16, 2021

Choose a reason for hiding this comment

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

这里需要说明下 boxes_num是整形, 以及支持的dtype

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 16, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@qingqing01 qingqing01 Sep 16, 2021

Choose a reason for hiding this comment

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

名字简短一些，roi_pool就行，去掉后面_c

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 16, 2021

Choose a reason for hiding this comment

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

done

@lyuwenyu lyuwenyu mentioned this pull request Sep 17, 2021
Merged
TCChenlong
TCChenlong previously approved these changes Sep 22, 2021
View reviewed changes
python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

Args 前有个空行

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

没有 Default: None

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

import paddle 前要加一行空行

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

Args: 前加个空行

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

import paddle 前有个空行

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

python/paddle/vision/ops.py Outdated
Copy link
Contributor

@TCChenlong TCChenlong Sep 15, 2021

Choose a reason for hiding this comment

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

Returns 前有个空行

Copy link
Contributor Author

@lyuwenyu lyuwenyu Sep 22, 2021

Choose a reason for hiding this comment

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

done

XiaoguangHu01
XiaoguangHu01 reviewed Sep 23, 2021
View reviewed changes
Copy link
Contributor

@XiaoguangHu01 XiaoguangHu01 left a comment

Choose a reason for hiding this comment

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

输入名称建议用x，对齐avd_pool和conv2d

@lyuwenyu
Copy link
Contributor Author

lyuwenyu commented Sep 23, 2021

输入名称建议用x，对齐avd_pool和conv2d

done

@lyuwenyu lyuwenyu force-pushed the roi_pool_L branch 2 times, most recently from 541813e to 8890a26 Compare September 26, 2021 12:10
Superjomn
Superjomn previously approved these changes Sep 26, 2021
View reviewed changes
Copy link
Contributor

@Superjomn Superjomn left a comment

Choose a reason for hiding this comment

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

LGTM

@lyuwenyu lyuwenyu closed this Sep 26, 2021
@lyuwenyu lyuwenyu reopened this Sep 26, 2021
@lyuwenyu lyuwenyu closed this Sep 26, 2021
@lyuwenyu lyuwenyu reopened this Sep 26, 2021
Superjomn
Superjomn approved these changes Sep 26, 2021
View reviewed changes
Copy link
Contributor

@Superjomn Superjomn left a comment

Choose a reason for hiding this comment

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

LGTM

@Superjomn Superjomn merged commit 6d62769 into PaddlePaddle:develop Sep 27, 2021
lyuwenyu added a commit to lyuwenyu/Paddle that referenced this pull request Sep 27, 2021
@lyuwenyu
Add roi pool (PaddlePaddle#35084)
26e81b7 
* add roi pool

* rename input as x
lanxianghit pushed a commit that referenced this pull request Sep 29, 2021
@lyuwenyu
Add roi pool (#35084) (#36154)
b0289de 
* add roi pool

* rename input as x
AnnaTrainingG pushed a commit to AnnaTrainingG/Paddle that referenced this pull request Sep 29, 2021
@lyuwenyu @AnnaTrainingG
Add roi pool (PaddlePaddle#35084)
34b0b7f 
* add roi pool

* rename input as x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qingqing01 qingqing01 qingqing01 left review comments

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 left review comments

@TCChenlong TCChenlong TCChenlong approved these changes

@jerrywgz jerrywgz Awaiting requested review from jerrywgz

@heavengate heavengate Awaiting requested review from heavengate

+1 more reviewer

@Superjomn Superjomn Superjomn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lyuwenyu @Superjomn @qingqing01 @TCChenlong @XiaoguangHu01