autoware.universe
autoware.universe copied to clipboard
feat(lidar_centerpoint): accelerated preprocessing for centerpoint
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
- Tested using data from the taxi
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.
- [x] I've confirmed the contribution guidelines.
- [x] The PR follows the pull request guidelines.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
- [ ] The PR follows the pull request guidelines.
- [ ] The PR has been properly tested.
- [ ] The PR has been reviewed by the code owners.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
- [ ] There are no open discussions or they are tracked via tickets.
- [ ] The PR is ready for merge.
After all checkboxes are checked, anyone who has write access can merge the PR.
@kminoda This is by no means urgent, but it is always nice to reduce the perception latency.
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 :pray:
@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.
@kminoda Regarding the evaluation I will look into it as soon as I can (got other stuff to deliver first)
@amadeuszsz Hi, would you help me out reviewing CUDA related code?
@kminoda Sure! I would like to make a review next week. Please let me know if it's urgent, I might change my priorities. I guess you can't add me as a reviewer yet. I added my account to the contributors sheet, so I hope we will be able assign me soon.
@amadeuszsz Thank you. Not an urgent task, so please review this when you have time.
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
@kminoda @YoshiRi @tzhong518 Since pointainting inherited some logic from centerpoint, and centerpoint now does things differently for GPU acceleration, pointpainting broke. For now, I just copied the logic from centerpoint before this PR and added it to the pointpainting package (the same optimization could not be applied easily).
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 :+1:
From my side, lidar_centerpoint
is ready to merge! 🚀 Please, confirm if the pointpainting
discussion will be solved and we can proceed to merge.
@kminoda I will wait until the message migration happens before attempting to merge this. In addition I want to learn to use the evaluator so I will be trying that as soon as I get a respite from other things :pray: (edit: now it is properly blocked xD)
@knzo25 Hi, would you fix the cppcheck error? (see cppcheck-differential result)
perception/lidar_centerpoint/include/lidar_centerpoint/network/tensorrt_wrapper.hpp:34:4: warning: inconclusive: Class 'TensorRTWrapper' which has virtual members does not have a virtual destructor. [virtualDestructor] ~TensorRTWrapper();
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 3.21%. Comparing base (
507e3f4
) to head (0a8986f
). Report is 65 commits behind head on main.
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
Flag | Coverage Δ | |
---|---|---|
differential | 3.21% <ø> (?) |
|
total | ? |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@knzo25 starting from this PR, The CI for build-main started failing:
- https://github.com/autowarefoundation/autoware.universe/actions/runs/9590002364/job/26444678737#step:15:22008
8: C++ exception with description "cudaErrorInsufficientDriver (35)@/__w/autoware.universe/autoware.universe/perception/lidar_centerpoint/include/lidar_centerpoint/cuda_utils.hpp#L80: CUDA driver version is insufficient for CUDA runtime version" thrown in the test body.
For some reason the has CI passed for the build-and-test-differential
cuda yet it fails the build-and-test
checks.