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] Support S3DIS data pre-processing and dataset class #433

Merged
merged 13 commits into from
Apr 23, 2021

Conversation

Wuziyi616
Copy link
Contributor

@Wuziyi616 Wuziyi616 commented Apr 12, 2021

  • Provide instructions to S3DIS data downloading
  • Provide scripts and meta_data for S3DIS data pre-processing
  • Add a S3DISSegDataset class
  • Add a config file for S3DIS data loading

@Wuziyi616
Copy link
Contributor Author

Wuziyi616 commented Apr 12, 2021

Different from other datasets like ScanNet, S3DIS doesn't have a train-val split! S3DIS has 6 areas of data (can think of them as 6 subsets, collected in different buildings/regions). The common setting is training on 5 of them, and testing on the remaining one. People often report an Area_5 test result, and an average result. The average result means the average mIoU of 6 experiments (leaving Area_1/2/.../6 respectively).

Therefore, in create_data.py, I generate one info file for one area, so that we can concat 5 to form a training dataset and use the remaining one for test dataset.

In s3dis_dataset.py, I write an inner dataset class (_S3DISSegDataset) for data loading on 1 area, and further write a wrapper class (S3DISSegDataset) to hold (concat) multiple areas.

@Wuziyi616 Wuziyi616 changed the title [Feature] Support S3DIS data download and pre-processing [Feature] Support S3DIS data pre-processing and dataset class Apr 12, 2021
@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #433 (3eff65b) into master (78c29c3) will increase coverage by 0.26%.
The diff coverage is 90.80%.

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

@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
+ Coverage   51.04%   51.31%   +0.26%     
==========================================
  Files         187      188       +1     
  Lines       13797    13884      +87     
  Branches     2238     2259      +21     
==========================================
+ Hits         7043     7124      +81     
- Misses       6277     6280       +3     
- Partials      477      480       +3     
Flag Coverage Δ
unittests 51.31% <90.80%> (+0.26%) ⬆️

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

Impacted Files Coverage Δ
mmdet3d/datasets/custom_3d_seg.py 65.66% <ø> (ø)
mmdet3d/datasets/scannet_dataset.py 94.56% <ø> (ø)
mmdet3d/datasets/s3dis_dataset.py 90.69% <90.69%> (ø)
mmdet3d/datasets/__init__.py 100.00% <100.00%> (ø)
mmdet3d/datasets/pipelines/transforms_3d.py 86.92% <0.00%> (+0.43%) ⬆️

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 2c29afa...bfadf26. Read the comment docs.

@Tai-Wang
Copy link
Member

Tai-Wang commented Apr 13, 2021

Please check all the docstring, there are still many nonstandard formats.
Next time we can split similar PR into several small ones.
It is so heavy to review and merge with so many things in the single one 0.0

@Wuziyi616
Copy link
Contributor Author

I just copy-paste the pre-processing code of S3DIS from other repo so it's dirty, I am sorry. I check it and find many unnecessary functions. I will remove and make the code cleaner.

Yes, next time I will split into small PRs. I apologize for that.

@Tai-Wang
Copy link
Member

I just copy-paste the pre-processing code of S3DIS from other repo so it's dirty, I am sorry. I check it and find many unnecessary functions. I will remove and make the code cleaner.

Yes, next time I will split into small PRs. I apologize for that.

OK, it doesn't matter. You have done a good job :)

@Wuziyi616 Wuziyi616 changed the title [Feature] Support S3DIS data pre-processing and dataset class [Feature] Support S3DIS data pre-processing and dataset class [WIP] Apr 14, 2021
@Wuziyi616 Wuziyi616 marked this pull request as draft April 14, 2021 03:39
@ZwwWayne ZwwWayne added the WIP label Apr 14, 2021
@Wuziyi616 Wuziyi616 changed the title [Feature] Support S3DIS data pre-processing and dataset class [WIP] [Feature] Support S3DIS data pre-processing and dataset class Apr 15, 2021
@Wuziyi616 Wuziyi616 marked this pull request as ready for review April 16, 2021 06:51
@Wuziyi616
Copy link
Contributor Author

Need to add pipeline support in S3DISSegDataset.show/evaluate() after PR#430 is merged.

data/s3dis/collect_indoor3d_data.py Show resolved Hide resolved
data/s3dis/collect_indoor3d_data.py Outdated Show resolved Hide resolved
data/s3dis/collect_indoor3d_data.py Outdated Show resolved Hide resolved
data/s3dis/collect_indoor3d_data.py Outdated Show resolved Hide resolved
data/s3dis/indoor3d_util.py Outdated Show resolved Hide resolved
data/s3dis/indoor3d_util.py Outdated Show resolved Hide resolved
mmdet3d/datasets/s3dis_dataset.py Show resolved Hide resolved
Copy link
Contributor Author

@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.

Ready for review now @Tai-Wang @ZwwWayne.

@Wuziyi616
Copy link
Contributor Author

Resolve conflicts.

@Wuziyi616 Wuziyi616 requested a review from ZwwWayne April 22, 2021 03:05
@ZwwWayne ZwwWayne merged commit a0090aa into open-mmlab:master Apr 23, 2021
@Wuziyi616 Wuziyi616 deleted the s3dis branch April 24, 2021 06:15
tpoisonooo pushed a commit to tpoisonooo/mmdetection3d that referenced this pull request Sep 5, 2022
* [refactor][API2.0]  Add onnx export and jit trace (open-mmlab#419)

* first commit

* add async call

* add new api onnx export and jit trace

* add decorator

* fix ci

* fix torchscript ci

* fix loader

* better pipemanager

* remove comment, better import

* add kwargs

* remove comment

* better pipeline manager

* remove print

* [Refactor][API2.0] Api partition calibration (open-mmlab#433)

* first commit

* add async call

* add new api onnx export and jit trace

* add decorator

* fix ci

* fix torchscript ci

* fix loader

* better pipemanager

* remove comment, better import

* add partition

* move calibration

* Better create_calib_table

* better deploy

* add kwargs

* remove comment

* better pipeline manager

* rename api, remove reduant variable, and misc

* [Refactor][API2.0] Api ncnn openvino (open-mmlab#435)

* first commit

* add async call

* add new api onnx export and jit trace

* add decorator

* fix ci

* fix torchscript ci

* fix loader

* better pipemanager

* remove comment, better import

* add ncnn api

* finish ncnn api

* add openvino support

* add kwargs

* remove comment

* better pipeline manager

* merge fix

* merge util and onnx2ncnn

* fix docstring

* [Refactor][API2.0] API for TensorRT (open-mmlab#519)

* first commit

* add async call

* add new api onnx export and jit trace

* add decorator

* fix ci

* fix torchscript ci

* fix loader

* better pipemanager

* remove comment, better import

* add partition

* move calibration

* Better create_calib_table

* better deploy

* add kwargs

* remove comment

* Add tensorrt API

* better pipeline manager

* add tensorrt new api

* remove print

* rename api, remove reduant variable, and misc

* add docstring

* [Refactor][API2.0] Api ppl other (open-mmlab#528)

* first commit

* add async call

* add new api onnx export and jit trace

* add decorator

* fix ci

* fix torchscript ci

* fix loader

* better pipemanager

* remove comment, better import

* add kwargs

* Add new APIS for pplnn sdk and misc

* remove comment

* better pipeline manager

* merge fix

* update tools/onnx2pplnn.py

* rename function
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