feat(object_lanelet_filter): add configurable margin for object lanel… #9240

sebekx · 2024-11-05T14:34:02Z

object_lanelet_filter

Description

Added configurable margin to object lanelet filter which extends area of non filtered objects.

With margin 3m:

How was this PR tested?

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

github-actions · 2024-11-05T14:34:30Z

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

You've checked our contribution guidelines.
Your PR follows our pull request guidelines.
All required CI checks pass before marking the PR ready for review.

YoshiRi · 2024-11-14T04:35:53Z

@badai-nguyen @technolojin Could you check this PR? (especially calculation cost)

codecov · 2024-11-21T04:52:44Z

Codecov Report

Attention: Patch coverage is 0% with 35 lines in your changes missing coverage. Please review.

Project coverage is 29.50%. Comparing base (0831075) to head (5bf3d37).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...t_validation/src/lanelet_filter/lanelet_filter.cpp	0.00%	34 Missing ⚠️
...t_validation/src/lanelet_filter/lanelet_filter.hpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9240      +/-   ##
==========================================
- Coverage   29.51%   29.50%   -0.01%     
==========================================
  Files        1442     1443       +1     
  Lines      108594   108629      +35     
  Branches    41502    41525      +23     
==========================================
  Hits        32055    32055              
- Misses      73418    73453      +35     
  Partials     3121     3121

Flag	Coverage Δ		*Carryforward flag
differential	`5.30% <0.00%> (?)`
total	`29.52% <ø> (+<0.01%)`	⬆️	Carriedforward from 0831075

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

a-maumau · 2024-11-21T08:24:11Z

@sebekx Hi, thank you for your PR!

I timed the processing time by the TimeKeeper.

vs. implementation

impl.	larger map	lanelet_extra_margin	avg. processing time
current impl.	-	N/A	~6.0 ms
current impl.	✓	N/A	~6.5 ms
this PR	-	0.0	~3.8 ms
this PR	✓	0.0	~4.0 ms
this PR	-	0.1	~12.6 ms
this PR	✓	0.1	~13.6 ms
this PR	-	0.5	~11.4ms
this PR	-	1.0	~10.3 ms
this PR	-	3.0	~8.5 ms
this PR	✓	3.0	~9.2 ms

case of `debug=true`

impl.	debug	lanelet_extra_margin	avg. processing time
this PR	true	0.0	~4.4 ms

"larger map" in the table indicates that the same rosbag with a larger map, so the size of the map is the only difference. The rows having '✓' is using the larger map.
debug, lanelet_extra_margin is the parameter in the config file.

The implementation of this PR is faster than the current one when the lanelet_extra_margin is set to 0.0 (disabled).
(It should be coming from the reduction of the method calls)

@sebekx since I am not familiar with the bg::strategy::buffer, do you know what kind of behavior is expected for distance_strategy(<negative value>)? Checking the lanelets with debug=true option in this PR, it seems it does not create the margin for the lanelets.

a-maumau · 2024-11-21T08:26:09Z

Also, it would be nice if you create the update PR for the config file in the autoware_launch too!

a-maumau · 2024-11-21T08:28:54Z

perception/autoware_detected_object_validation/src/lanelet_filter/lanelet_filter.cpp

+  viz_pub_ =
+    this->create_publisher<visualization_msgs::msg::MarkerArray>("~/debug/marker", rclcpp::QoS{1});
+}


It might be better to create the publisher when the filter_settings_.debug == true since it only be used in publishDebugMarkers that is also only get called when filter_settings_.debug == true.

Added code that creates publisher only whether there is filter_settings_.debug == true.

a-maumau · 2024-11-21T08:35:17Z

perception/autoware_detected_object_validation/src/lanelet_filter/lanelet_filter.cpp

+  filter_settings_.debug = declare_parameter<bool>("filter_settings.debug");
+  filter_settings_.lanelet_extra_margin =
+    declare_parameter<double>("filter_settings.lanelet_extra_margin");
+  //


remove //

I think it still exist.

sorry, now should be removed

a-maumau · 2024-11-22T01:00:22Z

@sebekx also, could you fix the error in the cppcheck result ?

sebekx · 2024-11-22T07:46:10Z

@sebekx since I am not familiar with the bg::strategy::buffer, do you know what kind of behavior is expected for distance_strategy(<negative value>)? Checking the lanelets with debug=true option in this PR, it seems it does not create the margin for the lanelets.

@a-maumau Hi, thanks for review. Normally it will make polygon smaller but in this implementation there is a condition to only expand polygons whether there is positive value of extra margin.

sebekx · 2024-11-22T07:50:16Z

Also, it would be nice if you create the update PR for the config file in the autoware_launch too!

there is pr

sebekx · 2024-11-22T09:12:22Z

@sebekx also, could you fix the error in the cppcheck result ?

fixed

a-maumau · 2024-11-22T09:37:15Z

Hi, thanks for review. Normally it will make polygon smaller but in this implementation there is a condition to only expand polygons whether there is positive value of extra margin.

Ah, I simply forgot that part, thank you.

I first thought shrinking might have some needs in some cases, but it would make some gaps between the lanelet connections, so this is reasonable.

perception/autoware_detected_object_validation/src/lanelet_filter/lanelet_filter.cpp

technolojin · 2024-11-25T02:44:00Z

Also, it would be nice if you create the update PR for the config file in the autoware_launch too!

there is pr

Can you kindly add this link and description to this PR description?

technolojin · 2024-11-25T04:13:26Z

I am going to test its performance.

technolojin · 2024-11-25T05:35:53Z

I measured processing time in 3 different situation

A. Before
B. After with lanelet_extra_margin of 3.0
C. After withlanelet_extra_margin of zero

A: 5ms in low, 10ms of high load
B. 8ms in low, 19ms of high load
C. 3ms in low, 2ms of high load

In conclusion, the performance of the same usage is improved. If the extra margin is added, it causes computational cost (as expected).

technolojin

LGTM

technolojin · 2024-11-25T05:37:11Z

@YoshiRi can you confirm this PR?

perception/autoware_detected_object_validation/config/object_lanelet_filter.param.yaml

YoshiRi

LGTM

perception/autoware_detected_object_validation/src/lanelet_filter/debug.cpp

Shin-kyoto

Thanks! LGTM

NOTE: I did NOT check this PR by running this code.

…et filter Signed-off-by: Sebastian Zęderowski <[email protected]> modified: perception/autoware_detected_object_validation/src/lanelet_filter/lanelet_filter.cpp Signed-off-by: Sebastian Zęderowski <[email protected]>

… empty in debug mode Signed-off-by: Sebastian Zęderowski <[email protected]>

Signed-off-by: Sebastian Zęderowski <[email protected]>

Signed-off-by: Taekjin LEE <[email protected]> Signed-off-by: Sebastian Zęderowski <[email protected]>

Signed-off-by: Sebastian Zęderowski <[email protected]>

…ter/debug.cpp Co-authored-by: Shintaro Tomie <[email protected]> Signed-off-by: Sebastian Zęderowski <[email protected]>

Signed-off-by: Sebastian Zęderowski <[email protected]>

technolojin · 2024-12-02T00:37:32Z

@sebekx Thank you for your contribution!

sebekx requested review from badai-nguyen, Shin-kyoto, miursh, YoshiRi and yukkysaito as code owners November 5, 2024 14:34

github-actions bot added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) tag:require-cuda-build-and-test labels Nov 5, 2024

sebekx force-pushed the feat/autoware_detected_object_validation branch 2 times, most recently from b34c04b to 8ab23a3 Compare November 5, 2024 14:36

sebekx mentioned this pull request Nov 5, 2024

feat(object_lanelet_filter): add configurable margin for object lanel… autowarefoundation/autoware_launch#1210

Merged

sebekx force-pushed the feat/autoware_detected_object_validation branch from 8ab23a3 to f5196e4 Compare November 5, 2024 14:46

technolojin self-assigned this Nov 19, 2024

technolojin added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Nov 21, 2024

a-maumau reviewed Nov 21, 2024

View reviewed changes

technolojin reviewed Nov 25, 2024

View reviewed changes

perception/autoware_detected_object_validation/src/lanelet_filter/lanelet_filter.cpp Outdated Show resolved Hide resolved

technolojin approved these changes Nov 25, 2024

View reviewed changes

Shin-kyoto requested changes Nov 28, 2024

View reviewed changes

perception/autoware_detected_object_validation/config/object_lanelet_filter.param.yaml Show resolved Hide resolved

YoshiRi approved these changes Nov 28, 2024

View reviewed changes

github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Nov 29, 2024

technolojin requested a review from Shin-kyoto November 29, 2024 09:21

sebekx force-pushed the feat/autoware_detected_object_validation branch from 1e78267 to 97b0fad Compare November 29, 2024 09:44

Shin-kyoto requested changes Nov 29, 2024

View reviewed changes

perception/autoware_detected_object_validation/src/lanelet_filter/debug.cpp Outdated Show resolved Hide resolved

sebekx force-pushed the feat/autoware_detected_object_validation branch from a823ef8 to 8de8bed Compare November 29, 2024 11:05

Shin-kyoto approved these changes Nov 29, 2024

View reviewed changes

sebekx force-pushed the feat/autoware_detected_object_validation branch from 8de8bed to b96939e Compare November 29, 2024 21:34

Sebastian Zęderowski and others added 10 commits November 30, 2024 11:11

feat(object_lanelet_filter): add condition to check wheter polygon is…

e232cca

… empty in debug mode Signed-off-by: Sebastian Zęderowski <[email protected]>

feat(object_lanelet_filter): fix cppcheck

23dfacd

Signed-off-by: Sebastian Zęderowski <[email protected]>

fix: brig back missing type definition

cbf6473

Signed-off-by: Taekjin LEE <[email protected]> Signed-off-by: Sebastian Zęderowski <[email protected]>

feat: add stop watch for processing time in object lanelet filter

c960639

Signed-off-by: Taekjin LEE <[email protected]> Signed-off-by: Sebastian Zęderowski <[email protected]>

feat(object_lanelet_filter): remove extra comment line

ccd48a3

Signed-off-by: Sebastian Zęderowski <[email protected]>

feat(_object_lanelet_filter): udpate readme

72401b7

Signed-off-by: Sebastian Zęderowski <[email protected]>

style(pre-commit): autofix

ebe2892

Signed-off-by: Sebastian Zęderowski <[email protected]>

Update perception/autoware_detected_object_validation/src/lanelet_fil…

40d2aef

…ter/debug.cpp Co-authored-by: Shintaro Tomie <[email protected]> Signed-off-by: Sebastian Zęderowski <[email protected]>

style(pre-commit): autofix

5bf3d37

Signed-off-by: Sebastian Zęderowski <[email protected]>

sebekx force-pushed the feat/autoware_detected_object_validation branch from b96939e to 5bf3d37 Compare November 30, 2024 10:11

technolojin merged commit 4479e94 into autowarefoundation:main Dec 2, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(object_lanelet_filter): add configurable margin for object lanel… #9240

feat(object_lanelet_filter): add configurable margin for object lanel… #9240

sebekx commented Nov 5, 2024 •

edited

Loading

github-actions bot commented Nov 5, 2024 •

edited

Loading

YoshiRi commented Nov 14, 2024

codecov bot commented Nov 21, 2024 •

edited

Loading

a-maumau commented Nov 21, 2024 •

edited

Loading

a-maumau commented Nov 21, 2024

a-maumau Nov 21, 2024

sebekx Nov 22, 2024

a-maumau Nov 21, 2024

sebekx Nov 22, 2024

a-maumau Nov 22, 2024

sebekx Nov 25, 2024

a-maumau commented Nov 22, 2024

sebekx commented Nov 22, 2024 •

edited

Loading

sebekx commented Nov 22, 2024

sebekx commented Nov 22, 2024

a-maumau commented Nov 22, 2024

technolojin commented Nov 25, 2024

technolojin commented Nov 25, 2024

technolojin commented Nov 25, 2024

technolojin left a comment

technolojin commented Nov 25, 2024

YoshiRi left a comment

Shin-kyoto left a comment

technolojin commented Dec 2, 2024

feat(object_lanelet_filter): add configurable margin for object lanel… #9240

feat(object_lanelet_filter): add configurable margin for object lanel… #9240

Conversation

sebekx commented Nov 5, 2024 • edited Loading

Description

Related links

How was this PR tested?

Notes for reviewers

Interface changes

Effects on system behavior

github-actions bot commented Nov 5, 2024 • edited Loading

YoshiRi commented Nov 14, 2024

codecov bot commented Nov 21, 2024 • edited Loading

Codecov Report

a-maumau commented Nov 21, 2024 • edited Loading

vs. implementation

case of debug=true

a-maumau commented Nov 21, 2024

a-maumau Nov 21, 2024

Choose a reason for hiding this comment

sebekx Nov 22, 2024

Choose a reason for hiding this comment

a-maumau Nov 21, 2024

Choose a reason for hiding this comment

sebekx Nov 22, 2024

Choose a reason for hiding this comment

a-maumau Nov 22, 2024

Choose a reason for hiding this comment

sebekx Nov 25, 2024

Choose a reason for hiding this comment

a-maumau commented Nov 22, 2024

sebekx commented Nov 22, 2024 • edited Loading

sebekx commented Nov 22, 2024

sebekx commented Nov 22, 2024

a-maumau commented Nov 22, 2024

technolojin commented Nov 25, 2024

technolojin commented Nov 25, 2024

technolojin commented Nov 25, 2024

technolojin left a comment

Choose a reason for hiding this comment

technolojin commented Nov 25, 2024

YoshiRi left a comment

Choose a reason for hiding this comment

Shin-kyoto left a comment

Choose a reason for hiding this comment

technolojin commented Dec 2, 2024

sebekx commented Nov 5, 2024 •

edited

Loading

github-actions bot commented Nov 5, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading

a-maumau commented Nov 21, 2024 •

edited

Loading

case of `debug=true`

sebekx commented Nov 22, 2024 •

edited

Loading