Feature/active dsg port #120
Conversation
nathanhhughes
left a comment
nathanhhughes
left a comment
There was a problem hiding this comment.
LGTM! Some reminders for stuff we talked about in person to eventually clean up
| * -------------------------------------------------------------------------- */ | ||
| #pragma once | ||
| #include <gtsam/nonlinear/Values.h> | ||
| #include <spark_dsg/dynamic_scene_graph.h> |
There was a problem hiding this comment.
Unneeded (handled by the dsg_types include)
src/backend/dsg_updater.cpp
Outdated
| target_dsg_->graph->mergeGraph(*source_graph_, merge_config); | ||
|
|
||
| std::vector<NodeId> active_nodes_to_restore; | ||
| for (auto& node : source_graph_->getLayer(DsgLayers::PLACES).nodes()) { |
There was a problem hiding this comment.
I guess I missed this when we were looking at this earlier, but this should ideally be for all layers I think (or for some configurable list of layers), the places aren't the only thing this can happen to, it's just what we noticed while we were debugging the place deformation
src/backend/dsg_updater.cpp
Outdated
| if (name == "surface_places") { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Reminder that we talked about moving this to be a flag in the update functors themselves instead of doing this here by hard-coding a functor name (that isn't tied to the actual layer the functor operates on)
There was a problem hiding this comment.
What do you want this interface to look like? The base UpdateFunctor now gets an exhaustive merge flag?
src/backend/dsg_updater.cpp
Outdated
| } while (num_applied > 0); | ||
| } | ||
| if (info->loop_closure_detected && hooks.merge) { | ||
| LOG(WARNING) << "Updating all merge attributes for " << name; |
There was a problem hiding this comment.
Probably worth downgrading the logging severity here
src/backend/dsg_updater.cpp
Outdated
|
|
||
| // NOTE: the followings fails with hundreds of lines of errors about templates and | ||
| // threads: | ||
| // launchCallbacks(cleanup_hooks, info, *source_graph_, target_dsg_.get()); |
There was a problem hiding this comment.
Anything passed by reference to a std::thread constructor gets copied, the arg needs to be raw / shared pointer for this to work
| #include "hydra/backend/update_frontiers_functor.h" | ||
|
|
||
| #include <config_utilities/config.h> | ||
| #include <spark_dsg/dynamic_scene_graph.h> |
GoldenZephyr
force-pushed
the
feature/active_dsg_port
branch
from
8a60e1d to
afcead2
Compare
nathanhhughes
force-pushed
the
develop
branch
from
c25a70b to
39ec703
Compare
nathanhhughes
force-pushed
the
feature/active_dsg_port
branch
from
afcead2 to
ec02400
Compare
@nathanhhughes This is an initial port of the merge tracker and cleanup functor interface changes. This compiles, but no idea yet if it actually runs. I am going to add the rest of the place and frontier changes in another branch on top of this.