Review abstract types and names used in IIF that is exported from DFG

[Affie]

To review

• InferenceVariable
• AbstractRelativeMinimize
• AbstractManifoldMinimize
• pack/unpackDistribution
• SamplableBelief
• AbstractPrior -> PriorObservation
• AbstractRelative -> RelativeObservation
• TODO fill in more..

other

• InferenceType used in SDK should be removed

[Affie]

1. I'm happy with AbstractPrior and AbstractRelative, but we can add Observation if need for clarity eg. AbstractRelativeObservation

2. The noun [pack/unpack]Distribution and SamplableBelief refers to the same thing as far as I can tell, so should we combine them? Distribution in SDK is currently a duplicate of PackedSamplableBelief and should be consolidated and deprecated.

3. AbstractRelativeMinimize and AbstractManifoldMinimize should maybe become traits to be expandable, eg. getSolverMethod(::Pose2Pose2) = ManifoldMinimization() with the default to AbstractManifoldMinimize - I think we are moving to only one method in any case? But traits open us up to change easier in the future.

4. Is InferenceVariable clear enough for new users (used in declaring variables eg. struct Position{N} <: InferenceVariable end) and does it match with the new State noun? InferenceType in the SDK refers to the factor type adding to the confusion. VariableType is used in the SDK for the same thing. Variable type is also confusing as we have a few variable types (VariableCompute, VariableDFG, etc.) . Maybe [Abstract]VariableStateType?

5. There is currently a mix of InferenceVariables with a dimension parameter and without, we should maybe just formalize to always have a parameter (or never as it might just be Position{N}), eg. Pose2 becomes Pose{2} with alias as well. InferenceVariable would become InferenceVariable{N<:Integer} where N is always the dimension.

[dehann]

I'm happy with AbstractPrior and AbstractRelative, but we can add Observation if need for clarity eg. AbstractRelativeObservation

+1 for adding Observation, but why not just use RelativeObservation and PriorObservation. I feel the abstract designator is already available, so don't really need to repeat it in the signature -- i.e. isabstracttype(RelativeObservation) --> true.

The noun [pack/unpack]Distribution and SamplableBelief refers to the same thing as far as I can tell, so should we combine them? Distribution in SDK is currently a duplicate of PackedSamplableBelief and should be consolidated and deprecated.

+1 to remove the duplication, how about SerdeDistribution?
serde is Rust speak, reduced from ser/de, reduced from serialization/deserialization. Rust has important library serde and serde_json. That "noun" has become pretty useful in my day-speak.

AbstractRelativeMinimize and AbstractManifoldMinimize should maybe become traits to be expandable, eg. getSolverMethod(::Pose2Pose2) = ManifoldMinimization() with the default to AbstractManifoldMinimize -

Yes, +1 agree.

I think we are moving to only one method in any case? But traits open us up to change easier in the future.

My experience has been that there will always be another optimization library popping up somewhere, so probably we'll end up with one default, but would like to keep the option open of rapidly integrating a new optimization library in the future.

Is InferenceVariable clear enough for new users (used in declaring variables eg. struct Position{N} <: InferenceVariable end) and does it match with the new State noun? InferenceType in the SDK refers to the factor type adding to the confusion. VariableType is used in the SDK for the same thing. Variable type is also confusing as we have a few variable types (VariableCompute, VariableDFG, etc.) . Maybe [Abstract]VariableStateType?

Oh, dang that is confusing.

So my mental model has been VariableDataLevel <: DataLevel, but even that is weak and we don't have those names written out yet either. Do we need a DataLevel name for these as explicit types today - i.e. VariableCompute <: VariableDataLevel? I'm starting to think yes we do (maybe it's already there). Similarly FactorDataLevel <: DataLevel.

Then, to help newcomers (and old school GNC folks) find their way in new code, how about variables use abstract type StateEstimateVariable end -- i.e. struct Position{N} <: StateEstimateVariable. That order lines up with much existing literature on specifically "state estimate [for position] variables". The optimization community literature has "decision variables", but that's probably not good for our use since we're likely to rather talk about "decision taken from" control loops / actions taken from perception estimates. Maybe/sorta for the trajectory planning aspects. Either way state/decision variables certainly screams that our old names are bad, i.e. we should remove InferenceType/InferenceVariable/VariableType; also since InferenceType is likely to be replaced by traits as per bullet item 3. above.

There is currently a mix of InferenceVariables with a dimension parameter and without, we should maybe just formalize to always have a parameter (or never as it might just be Position{N}), eg. Pose2 becomes Pose{2} with alias as well.

yes, +1 for Position{N::Int}, Pose{N::Int}, etc.

InferenceVariable would become InferenceVariable{N<:Integer} where N is always the dimen[s]ion.

First reaction is no on this one, let's first get to one abstract type StateEstimateVariable -- I'm not sure we should bake case by case dimensionality into the abstract type.

---

TL;DR; Recap: Pose{N::Int} <: StateEstimateVariable, PriorPosition{N::Int} <: PriorObservation. Non-Julia getVariable returns VariableDFG <: VariableDataLevel, and logic similarly applies over all cases FactorDataLevel/VariableDataLevel <: DataLevel.

I'm also reading these as being defined in DFG, and SDK.jl imports them -- i.e. we are trying to remove all abstract types from SDK.jl and collect them in DFG as best able? Similarly, IIF just imports them from DFG. After this round, next might be sorting out abstracts in AMP too.