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): reduce memory usage of elevation_map_loader #2571

Merged

Conversation

Shin-kyoto
Copy link
Contributor

@Shin-kyoto Shin-kyoto commented Dec 22, 2022

Description

  • When you use large pointcloud map, elevation_map_loader use so much amount of memory and keep it after making elevation_map. So in this PR, remove unused object from member of ElevationMapLoaderNode.

  • Before PR, 10GB memory is allocated after generating elevation_map. (Elevation_map is generated in the memory peak with 13+αGB.)
    before_PR_

  • After PR, 1+αGB memory is allocated after generating elevation_map. (Elevation_map is generated in the memory peak with 12GB.)
    afterPR

Related links

JIRA link: TIERIV INTERNAL LINK

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.

@Shin-kyoto Shin-kyoto requested review from kosuke55, taichiH and a team as code owners December 22, 2022 15:18
@github-actions github-actions bot added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Dec 22, 2022
@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Base: 11.39% // Head: 10.54% // Decreases project coverage by -0.85% ⚠️

Coverage data is based on head (38d78b0) compared to base (33ecdb5).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2571      +/-   ##
==========================================
- Coverage   11.39%   10.54%   -0.86%     
==========================================
  Files        1279     1267      -12     
  Lines       89170    87680    -1490     
  Branches    23587    20938    -2649     
==========================================
- Hits        10161     9242     -919     
- Misses      68239    68573     +334     
+ Partials    10770     9865     -905     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 10.55% <0.00%> (-0.85%) ⬇️ Carriedforward from 4336653

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

Impacted Files Coverage Δ
..._localizer/include/ekf_localizer/ekf_localizer.hpp 0.00% <ø> (ø)
localization/ekf_localizer/src/ekf_localizer.cpp 0.00% <ø> (ø)
...elevation_map_loader/elevation_map_loader_node.hpp 0.00% <ø> (ø)
...ation_map_loader/src/elevation_map_loader_node.cpp 0.00% <0.00%> (ø)
...r/scene_module/avoidance/avoidance_module_data.hpp 0.00% <ø> (ø)
...ne_module/lane_following/lane_following_module.hpp 0.00% <ø> (ø)
...nner/scene_module/pull_out/pull_out_parameters.hpp 0.00% <ø> (ø)
...er/scene_module/pull_over/pull_over_parameters.hpp 0.00% <ø> (ø)
...nner/scene_module/side_shift/side_shift_module.hpp 0.00% <ø> (ø)
...or_path_planner/src/behavior_path_planner_node.cpp 0.18% <0.00%> (+0.01%) ⬆️
... and 229 more

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Shin-kyoto Shin-kyoto changed the title feat: reduce memory usage of elevation_map_loader feat(elevation_map_loader): reduce memory usage of elevation_map_loader Dec 22, 2022
Comment on lines 76 to 77
void createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader_);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader_);
void createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader);

Comment on lines 179 to 181
pcl::shared_ptr<grid_map::GridMapPclLoader> grid_map_pcl_loader_ =
pcl::make_shared<grid_map::GridMapPclLoader>(grid_map_logger);
grid_map_pcl_loader_->loadParameters(this->param_file_path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pcl::shared_ptr<grid_map::GridMapPclLoader> grid_map_pcl_loader_ =
pcl::make_shared<grid_map::GridMapPclLoader>(grid_map_logger);
grid_map_pcl_loader_->loadParameters(this->param_file_path);
pcl::shared_ptr<grid_map::GridMapPclLoader> grid_map_pcl_loader =
pcl::make_shared<grid_map::GridMapPclLoader>(grid_map_logger);
grid_map_pcl_loader->loadParameters(param_file_path_);

@@ -187,15 +189,16 @@ void ElevationMapLoaderNode::createElevationMap()
} else {
grid_map_pcl_loader_->setInputCloud(data_manager_.map_pcl_ptr_);
}
createElevationMapFromPointcloud();
createElevationMapFromPointcloud(grid_map_pcl_loader_);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
createElevationMapFromPointcloud(grid_map_pcl_loader_);
createElevationMapFromPointcloud(grid_map_pcl_loader);

@@ -187,15 +189,16 @@ void ElevationMapLoaderNode::createElevationMap()
} else {
grid_map_pcl_loader_->setInputCloud(data_manager_.map_pcl_ptr_);
}
createElevationMapFromPointcloud();
createElevationMapFromPointcloud(grid_map_pcl_loader_);
elevation_map_ = grid_map_pcl_loader_->getGridMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
elevation_map_ = grid_map_pcl_loader_->getGridMap();
elevation_map_ = grid_map_pcl_loader->getGridMap();

Comment on lines 200 to 201
void ElevationMapLoaderNode::createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader_)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
void ElevationMapLoaderNode::createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader_)
void ElevationMapLoaderNode::createElevationMapFromPointcloud(
const pcl::shared_ptr<grid_map::GridMapPclLoader> & grid_map_pcl_loader)

@@ -97,7 +98,7 @@ class ElevationMapLoaderNode : public rclcpp::Node
bool use_inpaint_;
float inpaint_radius_;
bool use_elevation_map_cloud_publisher_;
pcl::shared_ptr<grid_map::GridMapPclLoader> grid_map_pcl_loader_;
std::string param_file_path;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::string param_file_path;
std::string param_file_path_;

Copy link
Contributor

Choose a reason for hiding this comment

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

or not making a member, using this->declare_parameter("param_file_path", "path_default"); in createElevationMap() may be good

@@ -50,7 +50,7 @@ ElevationMapLoaderNode::ElevationMapLoaderNode(const rclcpp::NodeOptions & optio
: Node("elevation_map_loader", options)
{
layer_name_ = this->declare_parameter("map_layer_name", std::string("elevation"));
std::string param_file_path = this->declare_parameter("param_file_path", "path_default");
this->param_file_path = this->declare_parameter("param_file_path", "path_default");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this->param_file_path = this->declare_parameter("param_file_path", "path_default");
param_file_path_ = this->declare_parameter("param_file_path", "path_default");

Copy link
Contributor

Choose a reason for hiding this comment

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

it is better to align with other variables.

@Shin-kyoto
Copy link
Contributor Author

@kosuke55
Thank you for your review! I fixed variables' name.

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.

thanks very much, I did not test but the code looks good.

@Shin-kyoto
Copy link
Contributor Author

thanks very much, I did not test but the code looks good.

  • I checked code by generating elevation map with sample_map.

  • Checked by zooming elevation_map and found no broken points.
    image

  • No large holes in elevation_map from top view.
    image

  • The elevation_map does not deviate significantly from the point cloud.
    image

  • And this PR does not modify main algorithm, so it is enough to check for major breakdowns. I visualized elevation_map and checked it, and there was no problem. So I merge this PR.

@Shin-kyoto Shin-kyoto merged commit 214912a into autowarefoundation:main Jan 12, 2023
Shin-kyoto added a commit to Shin-kyoto/autoware.universe that referenced this pull request Feb 15, 2023
…er (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]>
h-ohta pushed a commit to tier4/autoware.universe that referenced this pull request Mar 24, 2023
…er (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]>
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
…er (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]>
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.

2 participants