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

[Feature] Add S3DIS dataset for 3D object detection #835

Merged
merged 9 commits into from
Aug 9, 2021

Conversation

filaPro
Copy link
Contributor

@filaPro filaPro commented Aug 4, 2021

Close #817.

@Wuziyi616 can you have a look about general structure? Then I can also add unit test and documentation. I'm also attaching visualization from browse_dataset.

Screenshot from 2021-08-04 16-30-41

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #835 (552773b) into master (fc9e0d9) will increase coverage by 0.05%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #835      +/-   ##
==========================================
+ Coverage   49.18%   49.23%   +0.05%     
==========================================
  Files         210      210              
  Lines       15986    16019      +33     
  Branches     2552     2556       +4     
==========================================
+ Hits         7862     7887      +25     
- Misses       7625     7630       +5     
- Partials      499      502       +3     
Flag Coverage Δ
unittests 49.23% <75.00%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
mmdet3d/datasets/s3dis_dataset.py 88.57% <74.19%> (-6.10%) ⬇️
mmdet3d/datasets/__init__.py 100.00% <100.00%> (ø)
mmdet3d/datasets/pipelines/formating.py 60.00% <0.00%> (ø)
mmdet3d/models/detectors/single_stage_mono3d.py 16.66% <0.00%> (ø)
mmdet3d/datasets/pipelines/transforms_3d.py 90.46% <0.00%> (+0.04%) ⬆️

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 fc9e0d9...552773b. 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.

Good job! @filaPro two questions:

  • Similar to ScanNet detection, S3DIS detection also doesn't involve yaw prediction right? So the boxes here are all axis-aligned.
  • Maybe we can move the two S3DIS detection datasets code to be above the two S3DIS segmentation datasets code? This follows scannet_dataset.py

The browse_dataset result is great! Only need to add some docstring and minor code structure refinement. Also unit tests are needed.

tools/data_converter/s3dis_data_utils.py Outdated Show resolved Hide resolved
tools/data_converter/s3dis_data_utils.py Outdated Show resolved Hide resolved
tools/data_converter/s3dis_data_utils.py Outdated Show resolved Hide resolved
tools/data_converter/s3dis_data_utils.py Outdated Show resolved Hide resolved
mmdet3d/datasets/s3dis_dataset.py Outdated Show resolved Hide resolved
mmdet3d/datasets/s3dis_dataset.py Outdated Show resolved Hide resolved
@Wuziyi616 Wuziyi616 changed the title [Enhance] Add s3dis dataset for 3d object detection [Feature] Add S3DIS dataset for 3D object detection Aug 5, 2021
@Wuziyi616 Wuziyi616 mentioned this pull request Aug 5, 2021
@Wuziyi616
Copy link
Contributor

I let @THU17cyz have a look here because this may involve our progress of refactoring coordinate system.

@Wuziyi616 Wuziyi616 requested a review from yezhen17 August 5, 2021 03:07
@filaPro
Copy link
Contributor Author

filaPro commented Aug 5, 2021

@Wuziyi616,

  • Yes this dataset doesn't have yaw angle.
  • Moved detection dataset to be above the segmentation one.

I also removed _S3DISDataset in favour of ConcatDataset. This also requires a fmall fix in browse_dataset. Do you think it is ok now?

configs/_base_/datasets/s3dis-3d-5class.py Show resolved Hide resolved
mmdet3d/datasets/s3dis_dataset.py Outdated Show resolved Hide resolved
mmdet3d/datasets/s3dis_dataset.py Outdated Show resolved Hide resolved
@yezhen17
Copy link
Collaborator

yezhen17 commented Aug 5, 2021

Great work, many thanks to @filaPro ! Similar to ScanNet det, there is no conflict with our ongoing coordinate system refactor.

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.

Almost ready to merge. Only need to add some unit tests.

mmdet3d/datasets/s3dis_dataset.py Outdated Show resolved Hide resolved
tools/misc/browse_dataset.py Outdated Show resolved Hide resolved
configs/_base_/datasets/s3dis-3d-5class.py Outdated Show resolved Hide resolved
configs/_base_/datasets/s3dis-3d-5class.py Outdated Show resolved Hide resolved
@filaPro
Copy link
Contributor Author

filaPro commented Aug 5, 2021

Don't quite understand lint problems, so revert yapf: disable back.

@Wuziyi616
Copy link
Contributor

Don't quite understand lint problems, so revert yapf: disable back.

This is because sometimes isort and yapf will have different behaviors in formatting some codes. You need to disable one of them to pass the linting check.

@yezhen17
Copy link
Collaborator

yezhen17 commented Aug 5, 2021

Ready to merge after unit tests are added

@filaPro
Copy link
Contributor Author

filaPro commented Aug 6, 2021

Also updated test data file for S3DIS.

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.

Good Job @filaPro! Cheers!

@ZwwWayne ZwwWayne merged commit 6d89995 into open-mmlab:master Aug 9, 2021
@filaPro filaPro deleted the s3dis branch August 14, 2021 06:40
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.

3D Object detection on S3DIS
4 participants