No place merging: a place is one image.

[kmactavish]

When a loop closure is detected, the generator likelihoods for that place should be updated with the newest measurement. What currently happens:

• The matched image is just not added to the map (quick and easy).
• Place generators are recomputed from the single measurement at every comparison (extra computation to avoid additional complexity and memory).

This issue will be used to discuss a design for storing place generators, and updating them with new measurements.

[kmactavish]

I think the following features would be useful:

• [ ] A Place will be a class that allows you to store intermediate data products.
  • This should be polymorphic, as different variants might have different data products.
  • Backwards compatibility: still able to compare two BOWs, but the intermediate data product might be a Place.
• [ ] A PlaceGraph will be used to store the graph of Place objects.
  • New/matched Place would connect to the previous Place.
  • This allows the motion model can traverse all adjacent places (both recent and old).
  • Comparing a new image would compare to every Place node in the PlaceGraph.
  • Confirmed matches would update a Place node.
  • Would lend itself to visualizing the complete graph.

[arrenglover]

I think this functionality would be useful. Originally (FAB-MAP 1.0) Mark attempted to update place models by altering the probability an object (e) is present given the location (L), but the results in the IJRR journal were achieved using the current 'visually static' locations. If memory serves me correctly, with FAB-MAP2.0 instead multiple images were softly linked with a single location and the probability of the location given an observation was dependent on all linked images. Having a similar system in OpenFABMAP would be nice, and your added 'graph' additions seem logical. (How similar does this start to become with CatSLAM though?)

The way I see this added functionality integrated is to have a new/separate class inherited from the base FabMap class, or possibly the FabMap2.0 class. I want to keep the current methods operating as they are - a replica of the algorithms presented in the papers. Any additional functionality should build on it without changing it. It should probably be in a separate advanced.cpp from the core FabMap.cpp.

[kmactavish]

Hmm, good points.

For the place storage, I think it might be better to refactor the generator inference into its own class which is constructed alongside FabMap. By default, the existing inference type would be created (default parameter for the constructor). Optionally, you could pass a different inference object to the FabMap constructor; this 'advanced' class could be defined in a separate cpp. I'll mock something up before implementing fully.

The place merging would definitely come before the graph for the motion model, and it seems better to revisit that once the first has been implemented. I think would still be far from CatSLAM, we make no attempt to use local geometry / odometry. It only makes a slight difference to the (very crude) motion model, by providing more paths for diffusion.