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

feat(lidar_apollo_segmentation_tvm): port package #1181

Merged

Conversation

ambroise-arm
Copy link
Contributor

@ambroise-arm ambroise-arm commented Jun 27, 2022

Description

merge #893 first

Port apollo_lidar_segmentation from Autoware.Auto, which uses TVM for the inference. Renamed to match the name of the same package that uses TensorRT.

Related links

Example usage of #628

Tests performed

Ran the unit test locally.

Notes for reviewers

Building

By default the packages don't get built. It requires neural networks from the neural_networks_provider package that only get downloaded with the DOWNLOAD_ARTIFACTS cmake variable set. See its documentation for more information.

Package name

There is already a lidar_apollo_instance_segmentation package that uses TensorRT. I've tried to use the same name with a "_tvm" suffix, but it gets too long and breaks the max line size so I've dropped the "instance" in the name. We may want to rename the current lidar_apollo_instance_segmentation to lidar_apollo_segmentation_tensorrt (or something similar) to align the naming.

Output

The output messages don't match the ones from lidar_apollo_instance_segmentation. It will probably require some follow-up work for them to be interchangeable. But there is no simulator pipeline to test it with at the moment.

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.

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

@ambroise-arm ambroise-arm changed the title Lidar apollo segmentation tvm feat(lidar_apollo_segmentation_tvm): port package Jun 27, 2022
@ambroise-arm ambroise-arm added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Jun 27, 2022
@codecov
Copy link

codecov bot commented Aug 10, 2022

Codecov Report

Base: 10.34% // Head: 10.27% // Decreases project coverage by -0.06% ⚠️

Coverage data is based on head (ecd09b8) compared to base (d75ea6e).
Patch has no changes to coverable lines.

❗ Current head ecd09b8 differs from pull request most recent head b7d8a3b. Consider uploading reports for the commit b7d8a3b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1181      +/-   ##
==========================================
- Coverage   10.34%   10.27%   -0.07%     
==========================================
  Files        1162     1154       -8     
  Lines       82729    82071     -658     
  Branches    19271    19102     -169     
==========================================
- Hits         8555     8436     -119     
+ Misses      64914    64492     -422     
+ Partials     9260     9143     -117     
Flag Coverage Δ *Carryforward flag
differential ∅ <ø> (?)
total 10.26% <ø> (-0.07%) ⬇️ Carriedforward from 69758af

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
.../behavior_path_planner/src/turn_signal_decider.cpp 0.00% <0.00%> (-40.97%) ⬇️
...lude/behavior_path_planner/turn_signal_decider.hpp 0.00% <0.00%> (-25.00%) ⬇️
...signal_processing/test/src/lowpass_filter_test.cpp 25.71% <0.00%> (-10.29%) ⬇️
...vehicle_model/sim_model_ideal_steer_acc_geared.cpp 64.15% <0.00%> (-9.44%) ⬇️
...lanner/include/behavior_path_planner/utilities.hpp 25.00% <0.00%> (-1.32%) ⬇️
perception/lidar_centerpoint/src/node.cpp 0.00% <0.00%> (ø)
localization/ekf_localizer/src/measurement.cpp 100.00% <0.00%> (ø)
localization/ndt_scan_matcher/src/util_func.cpp 0.00% <0.00%> (ø)
planning/behavior_velocity_planner/src/node.cpp 0.41% <0.00%> (ø)
localization/ekf_localizer/src/ekf_localizer.cpp 0.00% <0.00%> (ø)
... and 42 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ambroise-arm ambroise-arm marked this pull request as ready for review August 10, 2022 11:51
@xmfcx xmfcx added this to the Bus ODD July-Aug Milestone milestone Aug 16, 2022
@xmfcx xmfcx added the status:help-wanted Assistance or contributors needed. label Aug 16, 2022
@xmfcx xmfcx mentioned this pull request Aug 16, 2022
5 tasks
@ambroise-arm
Copy link
Contributor Author

Thanks for the review! I am working on addressing the comments.

@ambroise-arm ambroise-arm force-pushed the lidar-apollo-segmentation-tvm branch 2 times, most recently from 73d6814 to 69758af Compare September 27, 2022 09:54
@ambroise-arm
Copy link
Contributor Author

The first force-push is for rebasing, the second force-push is the actual change.
I've tested the new output with the rosbag demo here. The quality of the detection is not quite as good as the TensorRT package, but I believe that can be addressed later.

@angry-crab
Copy link
Contributor

angry-crab commented Sep 28, 2022

Thank you for the update! I tried to run a rosbag test today but my environment was somehow broken and I'm still trying to fix it. Since you were able to run the test, this pr is good to go. As for quality of detection, I think we could open up another issue for that.

Port apollo_lidar_segmentation from Autoware.Auto, changing the name to
align with the TierIV package using TensorRT.

Issue-Id: SCM-4493
Change-Id: If2bd78d473a0044d06fa42658766ffe9177a59a2
Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>
Copy link
Contributor

@angry-crab angry-crab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ambroise-arm
Copy link
Contributor Author

I have created an issue to track the difference in detection quality.
Can you merge the PR? I don't have write access.

@angry-crab angry-crab enabled auto-merge (squash) September 30, 2022 00:05
@angry-crab angry-crab merged commit 6e5c69a into autowarefoundation:main Sep 30, 2022
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
Port apollo_lidar_segmentation from Autoware.Auto, changing the name to
align with the TierIV package using TensorRT.

Issue-Id: SCM-4493
Change-Id: If2bd78d473a0044d06fa42658766ffe9177a59a2
Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>

Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>
Co-authored-by: Luca Foschiani <[email protected]>
Co-authored-by: Xinyu Wang <[email protected]>
boyali referenced this pull request in boyali/autoware.universe Oct 3, 2022
Port apollo_lidar_segmentation from Autoware.Auto, changing the name to
align with the TierIV package using TensorRT.

Issue-Id: SCM-4493
Change-Id: If2bd78d473a0044d06fa42658766ffe9177a59a2
Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>

Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>
Co-authored-by: Luca Foschiani <[email protected]>
Co-authored-by: Xinyu Wang <[email protected]>
boyali referenced this pull request in boyali/autoware.universe Oct 19, 2022
Port apollo_lidar_segmentation from Autoware.Auto, changing the name to
align with the TierIV package using TensorRT.

Issue-Id: SCM-4493
Change-Id: If2bd78d473a0044d06fa42658766ffe9177a59a2
Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>

Signed-off-by: Luca Foschiani <[email protected]>
Signed-off-by: Ambroise Vincent <[email protected]>
Co-authored-by: Luca Foschiani <[email protected]>
Co-authored-by: Xinyu Wang <[email protected]>
@angry-crab angry-crab mentioned this pull request Dec 9, 2022
7 tasks
technolojin pushed a commit to technolojin/autoware.universe that referenced this pull request Apr 9, 2024
…drivable-area

fix(drivable_area_expansion): fix invalid access (autowarefoundation#6566)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned) status:help-wanted Assistance or contributors needed.
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants