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

fix(lidar_centerpoint): updated the config file for the centerpoint tiny v2 #3096

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

knzo25
Copy link
Contributor

@knzo25 knzo25 commented Mar 16, 2023

Description

When I uploaded the centerpoint tiny v2 I missed uploading the updated config file, which caused errors and an unnecessary bad performance.

The new file was tested in the environments in which the error arose

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.

Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
@knzo25 knzo25 requested a review from yukke42 as a code owner March 16, 2023 15:44
@knzo25 knzo25 self-assigned this Mar 16, 2023
@knzo25 knzo25 requested a review from a team as a code owner March 16, 2023 15:44
@github-actions github-actions bot added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Mar 16, 2023
@knzo25 knzo25 changed the title fix(lidar_centerpoint) fix(lidar_centerpoint): updated the config file for the centerpoint tiny v2 Mar 16, 2023
Copy link
Contributor

@shmpwk shmpwk left a comment

Choose a reason for hiding this comment

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

I tested this more than 3 times in logging simulator and AWSIM simulator.
Without this PR, my centerpoint tiny v2 died.
With this PR, I confirmed everything works fine!

@taikitanaka3
Copy link
Contributor

@yukke42
can you review this PR?

Copy link
Contributor

@yukke42 yukke42 left a comment

Choose a reason for hiding this comment

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

LGTM

@yukke42
Copy link
Contributor

yukke42 commented Mar 17, 2023

@shmpwk Thank you for cheking it to work.

@shmpwk shmpwk merged commit c1b024e into autowarefoundation:main Mar 17, 2023
@lexavtanke
Copy link
Contributor

Hello,
Sorry to mention, but It seem to me that there is a issue with this configuration with demo bag data in logging simulator. When I use it lidar_centerpoint can't produce bounding boxes and gives this warning:

[lidar_centerpoint_node-41] [WARN] [1679402682.492713175] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402682.707407694] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402682.898299740] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402683.322440747] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402683.513103993] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402683.734927234] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402683.937108600] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402684.148641184] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402684.347404019] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402684.544896785] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402684.767671966] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402684.976230420] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402685.401099385] [lidar_centerpoint]: No detected boxes.
[lidar_centerpoint_node-41] [WARN] [1679402685.630374434] [lidar_centerpoint]: No detected boxes.

Screenshot from 2023-03-21 21-06-05

I've started autoware loggin simulator with:
os2 launch autoware_launch logging_simulator.launch.xml map_path:=$HOME/autoware_map/sample-map-rosbag vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit planning:=false

And with this command I've started playing ros2 bag:
ros2 bag play ~/autoware_map/sample-rosbag/sample.db3 -s sqlite3 -r 0.5 --loop

It happens on the main branch of autoware.universe. Here is the video. If I change the config file to its previous stage it works fine and produce bounding boxes. Here is the video.

Actually centerpoint still throws this warning as you can see in the end of the video but not so frequently.

@lexavtanke
Copy link
Contributor

Also I dug a little bit deeper and this change cause it point_cloud_range: [-76.8, -76.8, -2.0, 76.8, 76.8, 4.0]

@yukke42
Copy link
Contributor

yukke42 commented Mar 23, 2023

@lexavtanke Thank you for the feedback. Cloud you try again after removing install/lidar_centerpoint and re-build?

@lexavtanke
Copy link
Contributor

@yukke42 Yes, sure. It works strange, very unstable. Here is the video.

centerpoint_rebuild.mp4

@lexavtanke
Copy link
Contributor

here is the video with old config file without rebuilding package.

centerpoint_old_config.mp4

@yukke42
Copy link
Contributor

yukke42 commented Mar 24, 2023

@lexavtanke I have checked only lidar_centerpoint node and there is no issue in my environment.
Cloud you let me know your PC spec? it look like that obejcts are detected but the outputs are so delayed.
image

@yukke42
Copy link
Contributor

yukke42 commented Mar 24, 2023

Or it might be an issue in another package. Please give me more time to check the behavior.

@lexavtanke
Copy link
Contributor

My PC spec
image

@knzo25
Copy link
Contributor Author

knzo25 commented Mar 24, 2023

@lexavtanke
Sorry, I was supposed to attend to this problem but have been doing some other tasks.
The model was trained used the ranges I pushed so the fact that it works better for other ranges should be a coincidence.
I will try in another environment with a clean install and get back to you

@knzo25
Copy link
Contributor Author

knzo25 commented Mar 27, 2023

@lexavtanke
I installed autoware in a clean environment closer to yours

image

It works without any issues here.
Could you try the following command with this data? (the ply file is the result in my environment)

ros2 launch lidar_centerpoint single_inference_lidar_centerpoint.launch.xml pcd_path:=test_pointcloud.pcd detections_path:=test_detections.ply

shmpwk pushed a commit that referenced this pull request Mar 31, 2023
…iny v2 (#3096)

Fixed the cfg file regarding centerpoint tiny

Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
shmpwk added a commit that referenced this pull request Mar 31, 2023
* fix(lidar_centerpoint): updated the config file for the centerpoint tiny v2 (#3096)

Fixed the cfg file regarding centerpoint tiny

Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>

* feat(traffic_light_map_based_detector): change the vibration parameter of the Traffic Light Detector (#3189)

Change the vibration parameter of the Traffic Light Detector. 

Improve the accuracy of yellow signal recognition in AWSIM.

* fix: use centerpoint_tiny to centerpoint

Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>

* revert: "fix: use centerpoint_tiny to centerpoint"

This reverts commit efcf726.

---------

Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
Signed-off-by: Shumpei Wakabayashi <shumpei.wakabayashi@tier4.jp>
Co-authored-by: Kenzo Lobos Tsunekawa <kenzo.lobos@tier4.jp>
Co-authored-by: mackierx111 <mackierx111@gmail.com>
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants