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.

Sign up for GitHub

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

Jump to bottom

feat(lidar_centerpoint_tvm): single inference lidar centerpoint node #3130

Merged
merged 6 commits into from
Apr 24, 2023
Merged

feat(lidar_centerpoint_tvm): single inference lidar centerpoint node #3130

merged 6 commits into from
Apr 24, 2023

Conversation

tohmae
Copy link
Contributor

@tohmae tohmae commented Mar 21, 2023

Description

I add single inference lidar centerpoint node to lidar_centerpoint_tvm.

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.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

@tohmae tohmae requested review from angry-crab and a team as code owners March 21, 2023 16:53
@github-actions github-actions bot added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Mar 21, 2023
@angry-crab
Copy link
Contributor

Thank you for the PR! Would you be able to confirm it is working or not?

@knzo25
Copy link
Contributor

knzo25 commented Mar 24, 2023

@angry-crab
Random question.

The ply generation and visualization code is shared with the lidar_centerpoint package. Is it better to duplicate code or put them somewhere common?

@angry-crab
Copy link
Contributor

angry-crab commented Mar 24, 2023 •
edited
Loading

Is it better to duplicate code or put them somewhere common?

Ideally, it should be shared. Is there any plan from the perception team to refactor the packages?

@knzo25
Copy link
Contributor

knzo25 commented Mar 24, 2023 •
edited
Loading

Not that I know, I just mentioned this because this PR is basically a copy of the scripts I made before.

@tohmae
Copy link
Contributor Author

tohmae commented Mar 26, 2023 •
edited
Loading

Thank you for your comments.
I ran Rosbag replay tutorial and recorded the /sensing/lidar/concatenated/pointcloud topic by rosbag2.
I extraced the pointcloud from the rosbag and save the pcd file and ply files were generated by single_inference_lidar_centerpoint_tvm and single_inference_lidar_centerpoint.

  • pcd
    pointcloud

  • ply generated by single_inference_lidar_centerpoint_tvm
    lidar_centerpoint_tvm

  • ply generated by single_inference_clidar_centerpoint
    lidar_centerpoint

@tohmae tohmae mentioned this pull request Apr 4, 2023
Closed
4 tasks
@tohmae tohmae force-pushed the add-lidar-centerpoint-tvm-single-node branch from 7bedd7f to 4d46cfe Compare April 23, 2023 15:54
@codecov
Copy link

codecov bot commented Apr 23, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (4af956b) 13.81% compared to head (7bedd7f) 13.82%.

❗ Current head 7bedd7f differs from pull request most recent head 4baa24d. Consider uploading reports for the commit 4baa24d to get more accurate results

Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #3130    +/-   ##
========================================
  Coverage   13.81%   13.82%            
========================================
  Files        1391     1391            
  Lines       97512    97395   -117     
  Branches    28871    28805    -66     
========================================
- Hits        13472    13465     -7     
+ Misses      69559    69462    -97     
+ Partials    14481    14468    -13
Flag Coverage Δ *Carryforward flag
differential ∅ <ø> (?)
total 13.82% <ø> (+<0.01%) ⬆️ Carriedforward from 09a2884

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

see 40 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tohmae
Copy link
Contributor Author

tohmae commented Apr 23, 2023

Thank you for your comments.
I removed unnecessary 4 parameters in single_inference_lidar_centerpoint_tvm.launch.xml.

angry-crab
angry-crab approved these changes Apr 24, 2023
View reviewed changes
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

@angry-crab angry-crab enabled auto-merge (squash) April 24, 2023 03:15
@angry-crab angry-crab merged commit aba954f into autowarefoundation:main Apr 24, 2023
@tier4-autoware-public-bot tier4-autoware-public-bot bot mentioned this pull request Apr 25, 2023
Merged
Mingyu1991 pushed a commit to Mingyu1991/autoware.universe that referenced this pull request Jun 26, 2023
@tohmae @pre-commit-ci
@angry-crab @Mingyu1991
feat(lidar_centerpoint_tvm): single inference lidar centerpoint node (a…
80d4e00 
…utowarefoundation#3130)

* add signle_inference_node to lidar_centerpoint_tvm

* style(pre-commit): autofix

* change python3 to python

* changes model_name in lidar_centerpoint_tvm.launch.xml from centerpoint_tiny to centerpoint

* remove unnecessary parameters in single_inference_lidar_centerpoint_tvm.launch.xml

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Xinyu Wang <93699235+angry-crab@users.noreply.github.com>
Signed-off-by: Mingyu Li <mingyu.li@tier4.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angry-crab angry-crab angry-crab approved these changes

Assignees
No one assigned
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tohmae @angry-crab @knzo25