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.

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

[Refactor]: Unified parameter initialization #622

Merged
merged 61 commits into from
Jul 1, 2021

Conversation

xiliu8006
Copy link
Contributor

@xiliu8006 xiliu8006 commented Jun 7, 2021

Motivation

Refactoring model initialization methods based MMCV. For more information, please refer to open-mmlab/mmcv#780

Dependency

Training results

Methods config Before refactor init_weight() After after init_weight()
3DSSD config 78.39(mAP) 78.30(mAP)
VoteNet config 59.07(AP@0.25), 35.77(AP@0.5) 60.01(AP@0.25), 36.77(AP@0.5)
PointPillars-car config 77.1(mAP) 77.12(mAP)
PartA2 config 79.16(mAP) 79.47(mAP)
SECOND config 79.07(mAP) 78.73(mAP)
imVoteNet config 64.04(AP@0.25) 63.82 (AP@0.25)
CenterPoint config 56.19(mAP), 64.43(NDS) 0.5627(mAP), 0.6459(NDS)
MVX-Net config 63.0(mAP) 62.50(mAP)
Regnet config 41.2(mAP), 55.2(NDS) 41.4(mAP), 54.3(NDS), batch_size=1
H3dnet config 66.43(AP@0.25), 48.01(AP@0.5) 65.76(AP@0.25), 47.32(AP@0.5)
SSN config 41.56(mAP), 54.83(NDS) 39.38(mAP), 54.01(NDS)
FCOS3D config 29.9(mAP), 37.3(NDS) 26.53(mAP), 34.07(NDS)

@ZwwWayne
Copy link
Collaborator

Need to resolve conflicts. FCOS3D seems to have low accuracy.

Copy link

@MeowZheng MeowZheng left a comment

Choose a reason for hiding this comment

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

After this pr merging, please check the log carefully.

'please use "init_cfg" instead')
self.init_cfg = dict(type='Pretrained', checkpoint=pretrained)
else:
self.init_cfg = dict(type='Kaiming', layer='Conv2d')

Choose a reason for hiding this comment

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

According to L66-67, why use Kaiming init here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we dont use the pretrained model, we want to use kaiming_init to init all Conv2d layer.

self.init_cfg = dict(
type='Kaiming',
layer='Conv2d',
override=dict(type='Kaiming', name='heatmap'))

Choose a reason for hiding this comment

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

I think override can be deleted, as you have init heatmap in init_weights.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I forget to delete it. thank you!

layer='Conv2d',
override=dict(
type='Kaiming',
name=self.cls_head[-1],

Choose a reason for hiding this comment

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

the value of name is attribute name. I suggest to rewrite init_weights for initializing cls_head.

@@ -17,7 +17,7 @@ def digit_version(version_str):
return digit_version


mmcv_minimum_version = '1.3.1'
mmcv_minimum_version = '1.3.5'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should set 1.3.8

@@ -27,17 +27,17 @@ def digit_version(version_str):
f'MMCV=={mmcv.__version__} is used but incompatible. ' \
f'Please install mmcv>={mmcv_minimum_version}, <={mmcv_maximum_version}.'

mmdet_minimum_version = '2.10.0'
mmdet_maximum_version = '2.11.0'
mmdet_minimum_version = '2.12.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be 2.14.0 then.

mmseg_minimum_version = '0.14.0'
mmseg_maximum_version = '0.14.0'
mmseg_minimum_version = '0.14.1'
mmseg_maximum_version = '0.15.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

may set 1.0.0 for now. Usually there is not many BC-breakings.

@codecov
Copy link

codecov bot commented Jun 30, 2021

Codecov Report

Merging #622 (2d3a88b) into master (57e470a) will decrease coverage by 2.21%.
The diff coverage is 72.92%.

❗ Current head 2d3a88b differs from pull request most recent head e142171. Consider uploading reports for the commit e142171 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #622      +/-   ##
==========================================
- Coverage   51.01%   48.80%   -2.22%     
==========================================
  Files         204      208       +4     
  Lines       15383    15858     +475     
  Branches     2488     2542      +54     
==========================================
- Hits         7848     7739     -109     
- Misses       7017     7625     +608     
+ Partials      518      494      -24     
Flag Coverage Δ
unittests 48.80% <72.92%> (-2.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmdet3d/models/dense_heads/fcos_mono3d_head.py 11.96% <ø> (-1.14%) ⬇️
mmdet3d/models/dense_heads/parta2_rpn_head.py 15.84% <ø> (ø)
mmdet3d/models/dense_heads/ssd_3d_head.py 14.28% <ø> (ø)
mmdet3d/models/detectors/centerpoint.py 15.66% <ø> (ø)
mmdet3d/models/detectors/dynamic_voxelnet.py 38.70% <ø> (ø)
mmdet3d/models/detectors/h3dnet.py 18.18% <ø> (ø)
mmdet3d/models/detectors/parta2.py 25.75% <ø> (ø)
mmdet3d/models/detectors/ssd3dnet.py 100.00% <ø> (ø)
mmdet3d/models/detectors/votenet.py 33.33% <ø> (ø)
mmdet3d/models/detectors/voxelnet.py 33.87% <ø> (-41.94%) ⬇️
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57e470a...e142171. Read the comment docs.

@@ -4,6 +4,15 @@ This document provides detailed descriptions of the BC-breaking changes in MMDet

## MMDetection3D 0.15.0

### Unified parameter initialization

To unify the parameter initialization in OpenMMLab projects, MMCV supports `BaseModule` that accepts `init_cfg` to allow the modules' parameters initialized in a flexible and unified manner. Now the users need to explicitly call `model.init_weights()` in the training script to initialize the model (as in [here](https://github.com/open-mmlab/mmdetection3d/blob/master/tools/train.py#L183), previously this was handled by the detector. Please refer to PR #378 for details.
Copy link
Collaborator

Choose a reason for hiding this comment

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

change #378 to #622

docs/compatibility.md Outdated Show resolved Hide resolved
mmdet3d/apis/inference.py Outdated Show resolved Hide resolved
@ZwwWayne ZwwWayne merged commit 0759041 into open-mmlab:master Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants