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] Support rotate points without bbox in GlobalRotScaleTrans #540

Merged
merged 4 commits into from
May 14, 2021

Conversation

Wuziyi616
Copy link
Contributor

In the current implementation of GlobalRotScaleTrans, we will rotate points only when bbox is presented in input_dict (here). However, in PC Seg, we need to rotate points without bbox. In this PR, I add a if condition for this situation. I also add some input check & perform params transformation in __init__ to save future time.

@Wuziyi616
Copy link
Contributor Author

Wuziyi616 commented May 12, 2021

I discover that in this unit test, someone has tested GlobalRotScaleTrans and the expected_points don't perform rotation. So, I wonder if don't rotate points when bbox is not presented is the desired behavior of this function?

@codecov
Copy link

codecov bot commented May 12, 2021

Codecov Report

Merging #540 (85dfa52) into master (43d7953) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
+ Coverage   50.79%   50.89%   +0.09%     
==========================================
  Files         192      192              
  Lines       14601    14609       +8     
  Branches     2378     2379       +1     
==========================================
+ Hits         7417     7435      +18     
+ Misses       6691     6683       -8     
+ Partials      493      491       -2     
Flag Coverage Δ
unittests 50.89% <100.00%> (+0.09%) ⬆️

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

Impacted Files Coverage Δ
mmdet3d/core/points/base_points.py 82.38% <100.00%> (+0.10%) ⬆️
mmdet3d/datasets/pipelines/transforms_3d.py 89.89% <100.00%> (+2.19%) ⬆️

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 43d7953...85dfa52. Read the comment docs.

@Tai-Wang
Copy link
Member

I discover that in this unit test, someone has tested GlobalRotScaleTrans and the expected_points don't perform rotation. So, I wonder if don't rotate points when bbox is not presented is the desired behavior of this function?

I guess it should be a legacy issue. We don't have a separate class for points before, so previously some transformations related to points are achieved with bounding boxes (a hack workaround). Similar problems exist here. Maybe we can gradually refactor these similar operations in the future.

@ZwwWayne ZwwWayne merged commit 9a30edf into open-mmlab:master May 14, 2021
@Wuziyi616 Wuziyi616 deleted the pipeline_rot_points branch May 14, 2021 05:43
tpoisonooo pushed a commit to tpoisonooo/mmdetection3d that referenced this pull request Sep 5, 2022
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