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

perf(behavior_path_planner): simplify calculateDistanceLimit for dynamic drivable area expansion #4163

Merged

Conversation

maxime-clem
Copy link
Contributor

Description

This PR reduces the cost of running the dynamic drivable area expansion.
Currently, the calculations are very heavy and do not scale well with the number of "uncrossable lines".
This PR improves the performances but do not completely solve the scaling issue.

Tests performed

Tested in Psim.

Effects on system behavior

Running the dynamic drivable area expansion is less expensive, but the precision is reduced and the drivable area may slightly cross some lines we do not want to cross (e.g., road borders).

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: Maxime CLEMENT <maxime.clement@tier4.jp>
Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>
Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>
@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Jul 4, 2023
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Patch coverage: 20.83% and project coverage change: -0.01 ⚠️

Comparison is base (47db336) 15.24% compared to head (931af21) 15.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4163      +/-   ##
==========================================
- Coverage   15.24%   15.24%   -0.01%     
==========================================
  Files        1465     1465              
  Lines      101622   101707      +85     
  Branches    31359    31423      +64     
==========================================
+ Hits        15497    15507      +10     
- Misses      69277    69331      +54     
- Partials    16848    16869      +21     
Flag Coverage Δ *Carryforward flag
differential 13.51% <20.83%> (?)
total 15.25% <ø> (+<0.01%) ⬆️ Carriedforward from 47db336

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

Impacted Files Coverage Δ
...rivable_area_expansion/drivable_area_expansion.cpp 50.00% <0.00%> (-1.86%) ⬇️
...er/src/utils/drivable_area_expansion/expansion.cpp 33.02% <0.00%> (+3.71%) ⬆️
...path_planner/test/test_drivable_area_expansion.cpp 33.13% <26.31%> (-0.87%) ⬇️

... and 2 files with indirect coverage changes

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

Copy link
Contributor

@tkimura4 tkimura4 left a comment

Choose a reason for hiding this comment

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

LGTM.

I checked the frequency of /planning/scenario_planning/lane_driving/behavior_planning/path_with_lane_id in the curved lane.
( I enabled dynamic_expansion and changed extra_footprint_offset from 0.5 to 3.0. )

On my PC (ThinkPad X1 Extreme Gen4), the frequency was 0.2Hz before PR merge, but it becomes >9.9Hz after PR merge.

@tkimura4 tkimura4 merged commit 4693a77 into autowarefoundation:main Jul 7, 2023
17 of 21 checks passed
@maxime-clem maxime-clem deleted the perf/dynamic_expansion-rm_within branch July 7, 2023 03:29
tkimura4 pushed a commit to tier4/autoware.universe that referenced this pull request Jul 7, 2023
…mic drivable area expansion (autowarefoundation#4163)

* Add a test for the calculateDistanceLimit function

Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>

* Remove use of boost::geometry::within to improve performance

Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>

* [TMP] run behavior_path_planner with gdb

Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>

* Revert "[TMP] run behavior_path_planner with gdb"

This reverts commit b291af4.

---------

Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>
tkimura4 added a commit to tier4/autoware.universe that referenced this pull request Jul 7, 2023
…mic drivable area expansion (autowarefoundation#4163) (#650)

* Add a test for the calculateDistanceLimit function



* Remove use of boost::geometry::within to improve performance



* [TMP] run behavior_path_planner with gdb



* Revert "[TMP] run behavior_path_planner with gdb"

This reverts commit b291af4.

---------

Signed-off-by: Maxime CLEMENT <maxime.clement@tier4.jp>
Co-authored-by: Maxime CLEMENT <78338830+maxime-clem@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants