Skip to content

Graph-Based Layer System for Dynamic Cost Updates#83

Merged
amock merged 33 commits into
naturerobots:feature/dynamic-cost-updatesfrom
JustusBraun:feature/dynamic-cost-updates
Aug 15, 2025
Merged

Graph-Based Layer System for Dynamic Cost Updates#83
amock merged 33 commits into
naturerobots:feature/dynamic-cost-updatesfrom
JustusBraun:feature/dynamic-cost-updates

Conversation

@JustusBraun

Copy link
Copy Markdown
Collaborator

This PR replaces the layer management system with a dependency graph to model how data flows between layers.
The combination of cost layers is now handled using combination layers, which combine the cost maps of multiple input layers in different ways. The CombinationLayer replicates the weighted average used by the old layer system and the MaxCombinationLayer uses the maximum cost of a vertex across all input layers. The layer used for planning and control can be configured using the MeshMap's default_layer parameter. The costs are copied from the layer into the MeshMap's dense cost buffer to allow for async access and updates and constant time lookup of vertex costs.

for processing incomming data e.g. PointClouds
It is enough to only check the two faces current_vh and nh have in
common. This greatly reduces the number of faces checked for each
neighbor vertex.
… change a layer should be from the layer itself
The dynamic inflation layer was used for protoyping performance
improvements. These improvements were now applied to the InflationLayer
so this layer is no longer needed.
@JustusBraun
JustusBraun requested review from Cakem1x and amock August 8, 2025 18:38
@JustusBraun

Copy link
Copy Markdown
Collaborator Author

I forgot to mention that these changes require the MeshVertexCostsSparseStamped from mesh_tools/feature/dynamic-cost-updates, which is why the build failed.

@amock amock self-assigned this Aug 11, 2025
We do not want to normalize the combined costs because this creates the
need to recompute/renormalize all costs when the cost range changes due
to a local update.

@amock amock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After resolving some of the remaining TODOs and getting the CI running again, this looks good to be merged into this repository’s feature branch. I would also suggest increasing the version of all packages to 2.1.0.

Since we have a lot of changes here, for the next step of merging into the main branch it would be good to have runtime tests. For example, a new tutorial package that uses the dynamic layer or something similar.

Comment thread mbf_mesh_nav/src/mbf_mesh_nav.cpp
@JustusBraun

Copy link
Copy Markdown
Collaborator Author

Since this PR changes the AbstractLayer interface I agree that we should increase the version number.

Because we are already changing the interface I would suggest changing the AbstractLayer::costs() and AbstractLayer::lethals() methods to return const references to the respective data.
I don't see the need for external code to be able to change the costs or lethal vertices stored in a layer and as far is I know this is not used anywhere.

/**
* @brief Returns a vertex map, which associates a cost to each vertex handle.
* If a vertex handle is not associated with a cost value, i.e. there is no value
* in the map, the mesh_map will use the default value from threshold().
* @return a vertex map containing floats.
*/
virtual lvr2::VertexMap<float>& costs() = 0;
/**
* @brief Returns a set of vertex handles which are associated with "lethal" obstacles.
* @return set of vertex handles which are associated with lethal obstalces.
*/
virtual std::set<lvr2::VertexHandle>& lethals() = 0;

@amock

amock commented Aug 13, 2025

Copy link
Copy Markdown
Member

I successfully tested this PR in combination with the "feature/dynamic-cost-updates" branch of the tutorials under Ubuntu 24 / ROS 2 Jazzy.

@amock
amock merged commit 9e887a3 into naturerobots:feature/dynamic-cost-updates Aug 15, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants