feat(common): archive tracked nodes absent from an update round (+ propagate archived attrs to the backend)#156
Draft
harelb wants to merge 3 commits into
Draft
Conversation
Object nodes arrived is_active=true and nothing ever cleared the flag, so Pairwise merge candidates (archived-only) were always empty and backend object merges could never fire (box_6/7: 144/144 active). Opt-in per layer via LayerTracker archive_missing (default false).
Second hop of the same gate: graph_builder's frontend->backend_graph mergeGraph also skipped archived-node attribute updates, so the extractor's image_folder still never left the frontend (box_10: hook saw empty attrs, 0 renames).
Same gate as the frontend hop: mergeGraph skips attribute updates for archived target copies, so attributes written after a node archives never reached the backend's odometric mirror.
nathanhhughes
left a comment
nathanhhughes
left a comment
Collaborator
There was a problem hiding this comment.
I'm skeptical that this is useful; this is what I use to control whether or not objects should be archived (it defaults false because it messes with how the original Hydra mesh-based objects work but should be true in your case). Two reasons why this should be somewhere else:
- Archiving nodes that are missing from an update constrains how the active window behaves and/or makes it impossible to have two sources updating the same layer (which is not the end of the world, but seems not worth it if we don't need to do it this way)
- The archival / active window logic of whatever is sending the graph update may not be the same as the archival / active window logic of the frontend, and the frontend should have final say about what should still be active
Comment on lines
+349
to
+353
| // archived nodes still receive attribute updates (e.g. the object extractor's | ||
| // image_folder lands after the track archives) that the backend must see | ||
| GraphMergeConfig merge_config; | ||
| merge_config.update_archived_attributes = true; | ||
| state_->backend_graph->graph->mergeGraph(*dsg_->graph, merge_config); |
Collaborator
There was a problem hiding this comment.
This is way more expensive for no benefit. mergeGraph copies in attributes while the destination is active, so the last update to attributes (when is_active is flipped to false) is always caught. This is actually how is_active flips from true to false in the backend (so a lot more things would be broken if this didn't work)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (two halves of one lifecycle gap)
1. Track-driven nodes never archive.
GraphUpdaterstampsis_active=trueon every add/update of a tracked node and nothing ever clears it (places/frontiers have explicit archival; track-driven layers don't). Consequence:Pairwise::candidatesfilters to!is_active, so backend merge proposals for such layers are structurally empty — object dedup can never fire. Measured on a 38-min indoor bag: 144/144 object nodes still active at shutdown, zero merges ever, in every run.2. Archived copies are frozen forever. Once a downstream copy of a node is inactive,
mergeGraphskips its attribute updates (scene_graph_layer.cpparchived gate) — and sinceis_activeitself only updates via that same clone, the copy can never be reactivated. Any attribute written after archival (e.g. an extractor finalizing a node after its track leaves the active window) silently never reaches the backend.Fix
LayerTracker::Config::archive_missing(default false — no behavior change unless opted in): tracked nodes absent from a layer's update round getis_active=false; a returning track re-activates via the normal update path. Unit tests included.mergeGraphcalls setGraphMergeConfig::update_archived_attributes = true, so late attribute updates reach the backend mirror regardless of archival state.Measured (fork, with
archive_missing: truefor objects)Under a real 235-edge loop-closure stream: 11 duplicate objects merged by the existing backend merge machinery (previously structurally impossible), map quality unchanged (objects-on-mesh median 0.11 m).
Notes: exposed a latent
MergeTrackercrash once merges started firing — fixed separately in #155. Orthogonal to #153/#154.