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

feat(elevation_map_loader): use polygon iterator to speed up #2885

Merged
merged 11 commits into from
Mar 22, 2023

Conversation

miursh
Copy link
Contributor

@miursh miursh commented Feb 14, 2023

Description

When use_lane_filter is true, elevation map loader makes inpaint mask only within the lane polygons.
This option could be useful for large size map which takes long time to inpaint.

Related links

TIERIV COMPANY INTERNAL LINK

Tests performed

We have confirmed below

  • The elevation value is completely same within lanelet if margin is larger than 1m
  • Calculation time is much shorter than inpainting the entire area. (About 4 hours to 3 mins)

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.

@github-actions github-actions bot added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Feb 14, 2023
@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.07 🎉

Comparison is base (8b95d2a) 11.88% compared to head (e1cf536) 11.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2885      +/-   ##
==========================================
+ Coverage   11.88%   11.95%   +0.07%     
==========================================
  Files        1336     1325      -11     
  Lines       93197    91893    -1304     
  Branches    24728    24578     -150     
==========================================
- Hits        11077    10988      -89     
+ Misses      70641    69502    -1139     
+ Partials    11479    11403      -76     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 11.96% <ø> (+0.07%) ⬆️ Carriedforward from 2d598cf

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

Impacted Files Coverage Δ
...elevation_map_loader/elevation_map_loader_node.hpp 0.00% <ø> (ø)
...ation_map_loader/src/elevation_map_loader_node.cpp 0.00% <0.00%> (ø)

... and 45 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.

@miursh miursh marked this pull request as ready for review February 14, 2023 15:39
@miursh miursh requested review from kosuke55, taichiH and a team as code owners February 14, 2023 15:39
Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

For better performances I recommend using the grid_map_utils::PolygonIterator (https://github.com/autowarefoundation/autoware.universe/tree/main/common/grid_map_utils).
You only need to add grid_map_utils to packages.xml and apply the suggested changes.

@github-actions github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Feb 15, 2023
@@ -324,7 +324,7 @@ def create_single_frame_outlier_filter_components(input_topic, output_topic, con
],
parameters=[
{
"use_lane_filter": False,
"use_lane_filter": True,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is debatable. The detection area is looking at pointcloud, but it is often defined outside of ll2 lanes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if we use lane filter for inpaint, the elevation value in cells including min_num_points_per_cell or more points outside of lanelet is calculated.
This PR influences only cells outside of lanelet that do not include min_num_points_per_cell points.

(And if we can expand Polygon of lanelet, we can include cells in detection_area outside of lanelet.)

Copy link
Contributor

@yukkysaito yukkysaito Feb 15, 2023

Choose a reason for hiding this comment

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

@Shin-kyoto I see. 👍 I was mistaken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yukkysaito Although, setting default to True is not good idea, I think. I will fix this.
And this is a bit confusing, is it better to move these parameters to ground_segmentation.param.yaml? What do you think?

KYabuuchi pushed a commit to KYabuuchi/autoware.universe that referenced this pull request Feb 16, 2023
@miursh miursh marked this pull request as draft March 13, 2023 01:23
@miursh miursh marked this pull request as ready for review March 15, 2023 06:28
Signed-off-by: Shunsuke Miura <[email protected]>
Copy link
Contributor

@Shin-kyoto Shin-kyoto left a comment

Choose a reason for hiding this comment

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

LGTM

@miursh
Copy link
Contributor Author

miursh commented Mar 20, 2023

@kosuke55 @taichiH Could you review this PR as codeowners?

@Shin-kyoto and I have already checked it works all right

Copy link
Contributor

@kosuke55 kosuke55 left a comment

Choose a reason for hiding this comment

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

LGTM

@miursh miursh merged commit 14739a0 into autowarefoundation:main Mar 22, 2023
@miursh miursh deleted the feat/use_polygon_iterator branch March 22, 2023 04:39
h-ohta pushed a commit to tier4/autoware.universe that referenced this pull request Mar 24, 2023
…efoundation#2885)

* use grid_map::PolygonIterator instead of grid_map::GridMapIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* formatting

Signed-off-by: Shunsuke Miura <[email protected]>

* use use_lane_filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* delete unused use-lane-filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* change use_lane_filter to True, clarify the scope

Signed-off-by: Shunsuke Miura <[email protected]>

* change to use grid_map_utils::PolygonIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* Add lane margin parameter

Signed-off-by: Shunsuke Miura <[email protected]>

* use boost geometry buffer to expand lanes

Signed-off-by: Shunsuke Miura <[email protected]>

* Change use_lane_filter param default to false

Signed-off-by: Shunsuke Miura <[email protected]>

* update README

Signed-off-by: Shunsuke Miura <[email protected]>

---------

Signed-off-by: Shunsuke Miura <[email protected]>
h-ohta added a commit to tier4/autoware.universe that referenced this pull request Mar 31, 2023
…arefoundation#2571, autowarefoundation#2885, #943) (#337)

* feat(elevation_map_loader): reduce memory usage of elevation_map_loader (autowarefoundation#2571)

* feat: reduce memory usage of elevation_map_loader

Signed-off-by: Shin-kyoto <[email protected]>

* chore: remove unnecessary comment

Signed-off-by: Shin-kyoto <[email protected]>

* fix: modify variables' name

Signed-off-by: Shin-kyoto <[email protected]>

Signed-off-by: Shin-kyoto <[email protected]>

* feat(elevation_map_loader): use polygon iterator to speed up (autowarefoundation#2885)

* use grid_map::PolygonIterator instead of grid_map::GridMapIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* formatting

Signed-off-by: Shunsuke Miura <[email protected]>

* use use_lane_filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* delete unused use-lane-filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* change use_lane_filter to True, clarify the scope

Signed-off-by: Shunsuke Miura <[email protected]>

* change to use grid_map_utils::PolygonIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* Add lane margin parameter

Signed-off-by: Shunsuke Miura <[email protected]>

* use boost geometry buffer to expand lanes

Signed-off-by: Shunsuke Miura <[email protected]>

* Change use_lane_filter param default to false

Signed-off-by: Shunsuke Miura <[email protected]>

* update README

Signed-off-by: Shunsuke Miura <[email protected]>

---------

Signed-off-by: Shunsuke Miura <[email protected]>

* perf(behavior_velocity_planner): add faster PolygonIterator (#943)

* Add grid_map_utils pkg with faster implementation of PolygonIterator

Signed-off-by: Maxime CLEMENT <[email protected]>

* suppress error

---------

Signed-off-by: Shin-kyoto <[email protected]>
Signed-off-by: Shunsuke Miura <[email protected]>
Signed-off-by: Maxime CLEMENT <[email protected]>
Co-authored-by: Shintaro Tomie <[email protected]>
Co-authored-by: Shunsuke Miura <[email protected]>
Co-authored-by: Maxime CLEMENT <[email protected]>
badai-nguyen pushed a commit to badai-nguyen/autoware.universe that referenced this pull request Apr 4, 2023
…efoundation#2885)

* use grid_map::PolygonIterator instead of grid_map::GridMapIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* formatting

Signed-off-by: Shunsuke Miura <[email protected]>

* use use_lane_filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* delete unused use-lane-filter option

Signed-off-by: Shunsuke Miura <[email protected]>

* change use_lane_filter to True, clarify the scope

Signed-off-by: Shunsuke Miura <[email protected]>

* change to use grid_map_utils::PolygonIterator

Signed-off-by: Shunsuke Miura <[email protected]>

* Add lane margin parameter

Signed-off-by: Shunsuke Miura <[email protected]>

* use boost geometry buffer to expand lanes

Signed-off-by: Shunsuke Miura <[email protected]>

* Change use_lane_filter param default to false

Signed-off-by: Shunsuke Miura <[email protected]>

* update README

Signed-off-by: Shunsuke Miura <[email protected]>

---------

Signed-off-by: Shunsuke Miura <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:launch Launch files, scripts and initialization tools. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) type:documentation Creating or refining documentation. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants