Skip to content
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

reimplement ResNeXt based on ResNet #40588

Merged
merged 4 commits into from
Apr 25, 2022
Merged

reimplement ResNeXt based on ResNet #40588

merged 4 commits into from
Apr 25, 2022

Conversation

SigureMo
Copy link
Member

@SigureMo SigureMo commented Mar 15, 2022 •
edited
Loading

PR types

Breaking changes

PR changes

APIs

Describe

通过复用 ResNet 结构来重新实现 ResNeXt，避免额外在 resnext.py 实现一遍相似的结构。

API changes

  • 相对于 develop
- ResNeXt(depth=50, cardinality=32, num_classes=1000, with_pool=True)
- ResNet(block, depth=50, width=64, num_classes=1000, with_pool=True)
+ ResNet(block, depth=50, width=64, num_classes=1000, with_pool=True, groups=1)
  • 相对于 release/2.2（如果从这来看的话，貌似没有兼容性问题）
- ResNet(block, depth=50, num_classes=1000, with_pool=True)
+ ResNet(block, depth=50, width=64, num_classes=1000, with_pool=True, groups=1)

@paddle-bot-old
Copy link

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

@SigureMo SigureMo mentioned this pull request Mar 15, 2022
Merged
5 tasks
@SigureMo SigureMo changed the title reimplementing ResNeXt based on ResNet reimplement ResNeXt based on ResNet Mar 15, 2022
@SigureMo SigureMo marked this pull request as ready for review March 19, 2022 19:32
@SigureMo SigureMo mentioned this pull request Mar 19, 2022
Merged
6 tasks
@paddle-bot-old
Copy link

PR格式检查通过，你的PR将接受Paddle专家以及开源社区的review，请及时关注PR动态。
The format inspection passed. Your PR will be reviewed by experts of Paddle and developers from the open-source community. Stay tuned.

LielinJiang
LielinJiang previously approved these changes Mar 22, 2022
View reviewed changes
@TCChenlong
Copy link
Contributor

CI显示示例代码有问题~ @SigureMo
image

@paddle-bot-old
Copy link

paddle-bot-old bot commented Apr 6, 2022

Sorry to inform you that dcf312d's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@SigureMo SigureMo force-pushed the re-impl-resnext-based-on-resnet branch from dcf312d to 9e11b17 Compare April 21, 2022 12:50
LielinJiang
LielinJiang previously approved these changes Apr 22, 2022
View reviewed changes
TCChenlong
TCChenlong previously approved these changes Apr 22, 2022
View reviewed changes
XiaoguangHu01
XiaoguangHu01 reviewed Apr 22, 2022
View reviewed changes
python/paddle/vision/models/resnet.py

"""

def __init__(self,
block,
depth=50,
width=64,
Copy link
Contributor

Choose a reason for hiding this comment

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

这里造成了ResNet API的不兼容，

  1. width 替换成width_per_group的原因式什么？
  2. groups=1 新增参数放到最后，确保兼容

Copy link
Contributor

Choose a reason for hiding this comment

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

这里width的名称要不就不改变吧，在文档中说明清楚。兼容性比较重要

Copy link
Member Author

Choose a reason for hiding this comment

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

好的好的，我去修改一下

@SigureMo SigureMo dismissed stale reviews from TCChenlong and LielinJiang via c8268f5 April 22, 2022 09:55
TCChenlong
TCChenlong approved these changes Apr 24, 2022
View reviewed changes
Copy link
Contributor

@TCChenlong TCChenlong left a comment

Choose a reason for hiding this comment

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

LGTM

XiaoguangHu01
XiaoguangHu01 approved these changes Apr 25, 2022
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.

LGTM

@TCChenlong
Copy link
Contributor

请将这个PR，以及 #40431 cherry-pick 到 release/2.3 @SigureMo

@LielinJiang LielinJiang merged commit ba4e7c7 into PaddlePaddle:develop Apr 25, 2022
@SigureMo
Copy link
Member Author

请将这个PR，以及 #40431 cherry-pick 到 release/2.3 @SigureMo

好哒～

@SigureMo SigureMo deleted the re-impl-resnext-based-on-resnet branch April 25, 2022 14:16
SigureMo added a commit to cattidea/Paddle that referenced this pull request Apr 25, 2022
@SigureMo
reimplement ResNeXt based on ResNet (PaddlePaddle#40588)
70c2a20 
* refactor resnext
@SigureMo SigureMo mentioned this pull request Apr 25, 2022
Merged
XiaoguangHu01 pushed a commit that referenced this pull request Apr 26, 2022
@SigureMo
[cherry-pick] refactor vision models (#42252)
40ac92b 
* reuse ConvNormActivation in some vision models (#40431)

* reuse ConvNormActivation in some vision models

* reimplement ResNeXt based on ResNet (#40588)

* refactor resnext
@Liyulingyue Liyulingyue mentioned this pull request Apr 29, 2022
Closed
@SigureMo SigureMo mentioned this pull request Jun 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LielinJiang LielinJiang LielinJiang approved these changes

@XiaoguangHu01 XiaoguangHu01 XiaoguangHu01 approved these changes

@TCChenlong TCChenlong TCChenlong approved these changes

Assignees
No one assigned
Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SigureMo @TCChenlong @XiaoguangHu01 @LielinJiang