[AMCL] reduce the amount of memory used #4426

facontidavide · 2024-06-12T16:38:02Z

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:

And after the changes:

The main changes are:

Changes in map_cell_t: reduction from 12 bytes to 5 bytes
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:

Check that any new parameters added are updated in docs.nav2.org
Check that any significant change is added to the migration guide
Check that any new features OR changes to existing behaviors are reflected in the tuning guide
Check that any new functions have Doxygen added
Check that any new features have test coverage
Check that any new plugins is added to the plugins page
If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

doisyg · 2024-06-12T16:48:50Z

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 · 2024-06-12T17:10:46Z

@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 · 2024-06-12T17:16:39Z

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 · 2024-06-12T17:18:19Z

@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).

Signed-off-by: Davide Faconti <[email protected]>

mergify · 2024-06-12T17:38:32Z

@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).

Signed-off-by: Davide Faconti <[email protected]>

mergify · 2024-06-12T17:49:19Z

@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

Otherwise LGTM. AMCL changes usually scare me since its been untouched for so long, but this is minor and I don't foresee any issues.

double->float there's a reason MPPI/Smac both use floats for everything, I found massive improvements in performance on it alone, but I haven't had the cycles to try to apply that to the full stack to see how much more it could be reduced (though, those are 2 of the heaviest operations outside of the costmap).

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

nav2_amcl/src/amcl_node.cpp

SteveMacenski · 2024-06-12T17:53:47Z

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

Signed-off-by: Davide Faconti <[email protected]>

SteveMacenski · 2024-06-12T18:47:01Z

Still a draft, should I merge this in?

facontidavide · 2024-06-12T19:56:09Z

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 · 2024-06-12T20:07:20Z

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

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

SteveMacenski · 2024-06-12T20:10:19Z

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 😉

facontidavide · 2024-06-12T20:12:17Z

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 😉

* iprove memory used by AMCL and add parameter "freespace_downsampling" Signed-off-by: Davide Faconti <[email protected]> * fix message and linting Signed-off-by: Davide Faconti <[email protected]> * add class member Signed-off-by: Davide Faconti <[email protected]> --------- Signed-off-by: Davide Faconti <[email protected]>

[AMCL] reduce the amount of memory used, backport of upstream (ros-navigation#4426)

* iprove memory used by AMCL and add parameter "freespace_downsampling" Signed-off-by: Davide Faconti <[email protected]> * fix message and linting Signed-off-by: Davide Faconti <[email protected]> * add class member Signed-off-by: Davide Faconti <[email protected]> --------- Signed-off-by: Davide Faconti <[email protected]>

This changes likelihood storage from double to float, inspired by ros-navigation/navigation2#4426 . As a side-finding, this fixes a bug in the `make_likelihood_field()` function which causes memory footprint to go through the roof for large maps. Basically, because we applied the `max_obstacle_distance` threshold **before** calculating the distance map, for distances larger than `max_obstacle_distance` the priority queue in `nearest_obstacle_distance_map()` was processing the map in a random fashion instead of the "advancing wavefront" that the algorithm is meant to do. This caused the priority queue length to be almost as large as the map itself. This change does not affect either the accuracy or the average CPU usage. --------- Signed-off-by: Gerardo Puga <[email protected]>

facontidavide force-pushed the dfaconti/amcl_memory_reduction branch from 8a9516d to 1d798cb Compare June 12, 2024 16:39

facontidavide changed the title ~~[AMCL] DRAFT: reduce the amount of memory~~ [AMCL] reduce the amount of memory used Jun 12, 2024

facontidavide force-pushed the dfaconti/amcl_memory_reduction branch from bc58877 to 2c548d7 Compare June 12, 2024 17:15

facontidavide marked this pull request as draft June 12, 2024 17:20

iprove memory used by AMCL and add parameter "freespace_downsampling"

728abf3

Signed-off-by: Davide Faconti <[email protected]>

facontidavide force-pushed the dfaconti/amcl_memory_reduction branch from 2c548d7 to 728abf3 Compare June 12, 2024 17:34

fix message and linting

fa8eae7

Signed-off-by: Davide Faconti <[email protected]>

SteveMacenski approved these changes Jun 12, 2024

View reviewed changes

nav2_amcl/src/amcl_node.cpp Show resolved Hide resolved

add class member

cef7419

Signed-off-by: Davide Faconti <[email protected]>

facontidavide marked this pull request as ready for review June 12, 2024 19:55

SteveMacenski merged commit 23ddd86 into ros-navigation:main Jun 12, 2024
9 of 10 checks passed

doisyg mentioned this pull request Jun 12, 2024

[AMCL] reduce the amount of memory used, backport of upstream (#4426) botsandus/navigation2#41

Merged

doisyg added a commit to botsandus/navigation2 that referenced this pull request Jun 14, 2024

Merge pull request #41 from botsandus/backport_AMCL_mem_opt

be7c678

[AMCL] reduce the amount of memory used, backport of upstream (ros-navigation#4426)

glpuga mentioned this pull request Sep 23, 2024

Fix memory usage problem in 2D likelihood sensor model Ekumen-OS/beluga#433

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AMCL] reduce the amount of memory used #4426

[AMCL] reduce the amount of memory used #4426

facontidavide commented Jun 12, 2024 •

edited

Loading

doisyg commented Jun 12, 2024 •

edited

Loading

mergify bot commented Jun 12, 2024

facontidavide commented Jun 12, 2024 •

edited

Loading

mergify bot commented Jun 12, 2024

mergify bot commented Jun 12, 2024

mergify bot commented Jun 12, 2024

SteveMacenski left a comment

SteveMacenski commented Jun 12, 2024

SteveMacenski commented Jun 12, 2024

facontidavide commented Jun 12, 2024

facontidavide commented Jun 12, 2024

SteveMacenski commented Jun 12, 2024 •

edited

Loading

facontidavide commented Jun 12, 2024 •

edited

Loading

[AMCL] reduce the amount of memory used #4426

[AMCL] reduce the amount of memory used #4426

Conversation

facontidavide commented Jun 12, 2024 • edited Loading

Basic Info

Description of contribution in a few bullet points

Description of documentation updates required from your changes

Future work that may be required in bullet points

For Maintainers:

doisyg commented Jun 12, 2024 • edited Loading

mergify bot commented Jun 12, 2024

facontidavide commented Jun 12, 2024 • edited Loading

mergify bot commented Jun 12, 2024

mergify bot commented Jun 12, 2024

mergify bot commented Jun 12, 2024

SteveMacenski left a comment

Choose a reason for hiding this comment

SteveMacenski commented Jun 12, 2024

SteveMacenski commented Jun 12, 2024

facontidavide commented Jun 12, 2024

facontidavide commented Jun 12, 2024

SteveMacenski commented Jun 12, 2024 • edited Loading

facontidavide commented Jun 12, 2024 • edited Loading

facontidavide commented Jun 12, 2024 •

edited

Loading

doisyg commented Jun 12, 2024 •

edited

Loading

facontidavide commented Jun 12, 2024 •

edited

Loading

SteveMacenski commented Jun 12, 2024 •

edited

Loading

facontidavide commented Jun 12, 2024 •

edited

Loading