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

[Enhance] Refactor init_weight #378

Closed
wants to merge 0 commits into from

Conversation

xiliu8006
Copy link
Contributor

@xiliu8006 xiliu8006 commented Mar 24, 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)

@CLAassistant
Copy link

CLAassistant commented Apr 7, 2021

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #378 (87b04f6) into master (cde515d) will decrease coverage by 1.73%.
The diff coverage is 71.77%.

❗ Current head 87b04f6 differs from pull request most recent head 8830d63. Consider uploading reports for the commit 8830d63 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   50.97%   49.24%   -1.74%     
==========================================
  Files         197      189       -8     
  Lines       15057    14415     -642     
  Branches     2445     2355      -90     
==========================================
- Hits         7675     7098     -577     
+ Misses       6878     6851      -27     
+ Partials      504      466      -38     
Flag Coverage Δ
unittests 49.24% <71.77%> (-1.74%) ⬇️

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

Impacted Files Coverage Δ
mmdet3d/apis/inference.py 38.09% <0.00%> (-15.53%) ⬇️
mmdet3d/models/dense_heads/parta2_rpn_head.py 15.84% <ø> (ø)
mmdet3d/models/dense_heads/ssd_3d_head.py 13.93% <ø> (-0.36%) ⬇️
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 84 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 c33d4ec...8830d63. Read the comment docs.

Copy link
Contributor

@Wuziyi616 Wuziyi616 left a comment

Choose a reason for hiding this comment

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

I didn't check the model code very carefully.

| 0.7.0 | mmdet>=2.5.0, <=2.11.0 | mmcv-full>=1.1.5, <=1.4|
| 0.6.0 | mmdet>=2.4.0, <=2.11.0 | mmcv-full>=1.1.3, <=1.2|
| 0.5.0 | 2.3.0 | mmcv-full==1.0.5|
| master | mmdet>=2.10.0 | mmcv-full>=1.3.2, <=1.4|
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to change min version of mmdet to 2.12.0 because some of our detectors are inherited from mmdet?

@@ -27,7 +27,7 @@ def digit_version(version_str):
f'Please install mmcv>={mmcv_minimum_version}, <={mmcv_maximum_version}.'

mmdet_minimum_version = '2.10.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Also change the mmdet min version here to 2.12.0?

docs/getting_started.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Wuziyi616 Wuziyi616 left a comment

Choose a reason for hiding this comment

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

I compare this with mmdet's PR and it seems good to me.

docs/getting_started.md Outdated Show resolved Hide resolved
super().__init__()
normalize_xyz=True),
init_cfg=None):
super().__init__(init_cfg=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

init_cfg=init_cfg

mmdet3d/models/roi_heads/bbox_heads/parta2_bbox_head.py Outdated Show resolved Hide resolved
@ZwwWayne
Copy link
Collaborator

Need to resolve conflicts.

@ZwwWayne
Copy link
Collaborator

This PR relies on open-mmlab/mmsegmentation#567

@@ -12,7 +12,7 @@ The required versions of MMCV, MMDetection and MMSegmentation for different vers

| MMDetection3D version | MMDetection version | MMSegmentation version | MMCV version |
|:-------------------:|:-------------------:|:-------------------:|:-------------------:|
| master | mmdet>=2.10.0, <=2.11.0| mmseg>=0.13.0 | mmcv-full>=1.3.1, <=1.4|
| master | mmdet>=2.12.0 | mmseg>=0.13.0 | mmcv-full>=1.2.4, <=1.4|
Copy link
Collaborator

Choose a reason for hiding this comment

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

should >=1.3.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry,we should >=1.3.2, because mmdet need mmcv-full version >= 1.3.2

@@ -26,16 +26,13 @@ from ..builder import VOXEL_ENCODERS


@VOXEL_ENCODERS.register_module()
class HardVFE(nn.Module):
class HardVFE(BaseModule):

def __init__(self, arg1, arg2):
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this init add the argument init_cfg?

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


mmcv_minimum_version = '1.3.1'
mmcv_minimum_version = '1.3.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

just use the newest one because we made BC-breaking anyway.

@ZwwWayne
Copy link
Collaborator

ZwwWayne commented Jun 6, 2021

Need to resolve conflicts and update compatibility doc.

@xiliu8006 xiliu8006 closed this Jun 7, 2021
tpoisonooo pushed a commit to tpoisonooo/mmdetection3d that referenced this pull request Sep 5, 2022
* fix ncnn docs`

* update 0216
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.

4 participants