fix(behavior_path_planner): change getLaneletSequence function parameter for large scale lanelet2 #5940
Conversation
StepTurtle
added
the
component:planning
StepTurtle
requested review from
rej55,
kosuke55,
kyoichi-sugahara,
satoshi-ota,
shmpwk,
TakaHoribe,
takayuki5168,
TomohitoAndo,
tkimura4 and
zulfaqar-azmi-t4
as code owners
StepTurtle
added
the
run:build-and-test-differential
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5940 +/- ##
=======================================
Coverage 14.78% 14.78%
=======================================
Files 1917 1917
Lines 132038 132029 -9
Branches 39228 39222 -6
=======================================
- Hits 19523 19522 -1
+ Misses 90726 90716 -10
- Partials 21789 21791 +2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@StepTurtle Thanks. I'll start review. |
|
This pull request has been automatically marked as stale because it has not had recent activity. |
…cause of the time consumption. Signed-off-by: Barış Zeren <bzeren1819@gmail.com>
mehmetdogru
force-pushed
the
fix/planner-manager-time-consumption
branch
from
ef31b37 to
b4875b4
Compare
There was a problem hiding this comment.
The change greatly improves the performance with large maps and I do not think it can cause any issue.
@satoshi-ota please approve if you think this change is fine.
|
@maxime-clem @StepTurtle Thanks! And, sorry for the late response... 🙏 Unfortunatelly, this change may causes a problem. The manager creates original reference path based on root lanelet (see here) and the lanelet is updated only when the ego changes the following lane. Then, I guess it will not be able to find proper nearest lanelet in following functions because the distance between ego and root lanelet is sometimes longer than if (lanelet::utils::query::getClosestLaneletWithConstrains(
lanelet_sequence, pose, &closest_lane, p.ego_nearest_dist_threshold,
p.ego_nearest_yaw_threshold)) {
return utils::getReferencePath(closest_lane, data);
}
if (lanelet::utils::query::getClosestLanelet(lanelet_sequence, pose, &closest_lane)) {
return utils::getReferencePath(closest_lane, data);
}
On the other hand, current logic is no acceptable for large scale map, so I want to fix this issue as well. If you have any good ideas, please let me know. The requirements of the logic are:
I guess there will be a good way... |
|
@satoshi-ota thank you for your response, you've explained it quite well. Actually, after what you said, I also believe that the change we are about to make may cause some problems. If it's okay with you, I can close the PR. Also, unfortunately, I don't have much knowledge about planning components, so I don't have any other ideas |
Description
When we use current planning pipeline with large scale lanelet2 maps, we investigate some time consumption. You can check following discussion:
When we use
std::numeric_limits<double>::max()as a parameter, it takes all forward path it waste a lot of time. Using normal numbers reduce time consumption.Tests performed
This is the video before change:
This is the video after changes:
You can see, the time consumption is reducing with this change
Effects on system behavior
Nothing
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.
After all checkboxes are checked, anyone who has write access can merge the PR.