-
Notifications
You must be signed in to change notification settings - Fork 668
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
feat(lidar_centerpoint): accelerated preprocessing for centerpoint #6989
feat(lidar_centerpoint): accelerated preprocessing for centerpoint #6989
Conversation
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
@kminoda if you do not feel comfortable reviewing this PR, Uetake-san, Hirabayashi-san, Tanaka-san, and Amadeusz are good candidates) |
@knzo25 Thank you! And yes, I am still catching up with this package (especially the CUDA part), so would be nice if someone else could review it as a co-reviewer 🙏 @amadeuszsz Hi, would you help me out reviewing CUDA related code? |
@knzo25 And also, would you execute Autoware Evaluator and verify that the basic scenarios would pass? Let me know if you don't know how to execute it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the awesome PR 🚀
perception/lidar_centerpoint/lib/preprocess/pointcloud_densification.cpp
Show resolved
Hide resolved
@kminoda |
@kminoda |
@amadeuszsz Thank you. Not an urgent task, so please review this when you have time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Highly performance improvement observed! 👍🏻
Apart of minor changes requested, please check issues on your desktop with large clouds size & pointer arithmetic which causes wrong detections.
perception/lidar_centerpoint/lib/preprocess/voxel_generator.cpp
Outdated
Show resolved
Hide resolved
perception/lidar_centerpoint/include/lidar_centerpoint/preprocess/pointcloud_densification.hpp
Outdated
Show resolved
Hide resolved
perception/lidar_centerpoint/include/lidar_centerpoint/preprocess/pointcloud_densification.hpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
…imum capacity Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Note to self: Pointpainting was using some of centerpoint parts as a base clase, so I will have to port some of the changes to pointpainting or duplicate logic for pointpainting |
… new centerpoint logic is incompatible Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
@kminoda @YoshiRi @tzhong518 If you also want pointpainting accelerated, let's us discuss / schedule it a later date please |
@knzo25 Thanks. Still haven't understood in depth, but just to confirm: we would anyway have to implement different logic for centerpoint and pointpainting, right? I vote for merging this PR independently from pointpainting's further acceleration 👍 |
..._based_fusion/include/image_projection_based_fusion/pointpainting_fusion/voxel_generator.hpp
Show resolved
Hide resolved
From my side, |
@kminoda |
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Conflicts: perception/lidar_centerpoint/include/lidar_centerpoint/detection_class_remapper.hpp perception/lidar_centerpoint/include/lidar_centerpoint/node.hpp perception/lidar_centerpoint/include/lidar_centerpoint/ros_utils.hpp
…e code that assumed row-major, implements check for data layout, and updates tests Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
@knzo25 Hi, would you fix the cppcheck error? (see cppcheck-differential result)
|
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6989 +/- ##
==========================================
- Coverage 14.84% 3.21% -11.63%
==========================================
Files 1999 126 -1873
Lines 139163 8294 -130869
Branches 43716 1402 -42314
==========================================
- Hits 20661 267 -20394
+ Misses 95731 7971 -87760
+ Partials 22771 56 -22715
☔ View full report in Codecov by Sentry. |
@knzo25 starting from this PR, The CI for build-main started failing:
For some reason the has CI passed for the |
…point (autowarefoundation#6989)" This reverts commit fffb149.
…utowarefoundation#6989) * feat: accelerated preprocessing for centerpoint Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: changed includes to quotes instead of brackets Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed variable name Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: addressed the case where the points in the cache is over the maximum capacity Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: fixed the offset of the output buffer in the kernel Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: reimplemented old logic in the pointpainting package since the new centerpoint logic is incompatible Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: eigen matrices are column major by default. this commit fixes the code that assumed row-major, implements check for data layout, and updates tests Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed missing virtual destructor Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> --------- Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Co-authored-by: kminoda <44218668+kminoda@users.noreply.github.com> Signed-off-by: Simon Eisenmann <simon.eisenmann@driveblocks.ai>
…6989) * feat: accelerated preprocessing for centerpoint Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: changed includes to quotes instead of brackets Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed variable name Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: addressed the case where the points in the cache is over the maximum capacity Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: fixed the offset of the output buffer in the kernel Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: reimplemented old logic in the pointpainting package since the new centerpoint logic is incompatible Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: eigen matrices are column major by default. this commit fixes the code that assumed row-major, implements check for data layout, and updates tests Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed missing virtual destructor Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> --------- Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Co-authored-by: kminoda <44218668+kminoda@users.noreply.github.com>
…utowarefoundation#6989) * feat: accelerated preprocessing for centerpoint Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: changed includes to quotes instead of brackets Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed variable name Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: addressed the case where the points in the cache is over the maximum capacity Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: fixed the offset of the output buffer in the kernel Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: reimplemented old logic in the pointpainting package since the new centerpoint logic is incompatible Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: eigen matrices are column major by default. this commit fixes the code that assumed row-major, implements check for data layout, and updates tests Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed missing virtual destructor Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> --------- Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Co-authored-by: kminoda <44218668+kminoda@users.noreply.github.com>
…utowarefoundation#6989) * feat: accelerated preprocessing for centerpoint Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: changed includes to quotes instead of brackets Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed variable name Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: addressed the case where the points in the cache is over the maximum capacity Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: fixed the offset of the output buffer in the kernel Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: reimplemented old logic in the pointpainting package since the new centerpoint logic is incompatible Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * fix: eigen matrices are column major by default. this commit fixes the code that assumed row-major, implements check for data layout, and updates tests Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> * chore: fixed missing virtual destructor Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> --------- Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp> Co-authored-by: kminoda <44218668+kminoda@users.noreply.github.com>
Description
While changing the point type to the new standard, realized that preprocessing was bottleneck in centerpoint.
Apologies to Amadeusz since he did almost the same already in transfusion, and I forgot until writing this PR but I also moved the preprocessing to cuda for gpu acceleration.
During tests in my desktop the detection time went from 16.9ms to 13.0ms, making preprocessing about ~1ms if we forcefully add syncs to measure the time.
Related links
Tests performed
Notes for reviewers
Interface changes
ROS Topic Changes
ROS Parameter Changes
Effects on system behavior
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.