ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.5k stars 1.27k forks source link

[AMCL] reduce the amount of memory used #4426

Closed facontidavide closed 3 months ago

facontidavide commented 3 months ago

Basic Info

Info Please fill out this column
Ticket(s) this addresses none
Primary OS tested on Ubuntu
Robotic platform tested on Simulation
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

This is a memory optimization that should not affect realtime perfomance, but will reduce considerably the amount of memory used by AMCL.

In our large map (200 Mb) this is the memory usage before the change:

2024-06-12_19-03_1

And after the changes:

2024-06-12_19-03

The main changes are:

  1. Changes in map_cell_t: reduction from 12 bytes to 5 bytes
  2. Add a new parameter to reduce the memory used by free_space_indices using 1/4th of the points (equally spaced)

Description of documentation updates required from your changes

Describe the new parameter "freespace_downsampling"


Future work that may be required in bullet points

To be discussed

For Maintainers:

doisyg commented 3 months ago

~~Noting that it corresponds to a new (smaller) maximum side size of 3.2 km for AMCL maps at a standard resolution of 0.05m/px. I think that it is fair and that we don't have any use cases of nav2 with maps that big. I expect other part of the system to fail before a map side . wdyt @SteveMacenski and @tonynajjar ?~~

mergify[bot] commented 3 months ago

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

facontidavide commented 3 months ago

UPDATE: sorry I don't understand why this PR include files outside nav2_amcl. I will clean it up

Also, I removed the limitation in terms of maximu size of the map, so ignore the 3.2 km remark from @doisyg

mergify[bot] commented 3 months ago

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] commented 3 months ago

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] commented 3 months ago

@facontidavide, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

SteveMacenski commented 3 months ago

freespace_downsampling_ needs to be a class member, that should get CI unblocked

SteveMacenski commented 3 months ago

Still a draft, should I merge this in?

facontidavide commented 3 months ago

if it is good for you, it is good for me. I will prepare tomorrow a PR for the documentation https://docs.nav2.org/configuration/packages/configuring-amcl.html?highlight=amcl

facontidavide commented 3 months ago

Party on! What's next for memory consumption? 😎

:smile: sparse costmaps ??? Not yet... not yet...

SteveMacenski commented 3 months ago

Please do update the configuration guide docs for this param. You can also announce it in the Iron->Jazzy migration guide file since Nav2 hasn't forked for Jazzy yet (still working on it..)

😄 sparse costmaps ??? Not yet... not yet...

Well before you go nutty there, lets discuss that. I have a broader plan for costmaps in the medium term future :wink:

facontidavide commented 3 months ago

Well before you go nutty there, lets discuss that. I have a broader plan for costmaps in the medium term future

Keep me in the loop! I am sure I can take some work out of your plate :wink: