Manifest VariableTypes (Singleton model) and new FMD

[GearsAD]

Issue Summary

History:

variableTypesofttype was added to DFGVariable DataLevel2 to solve two problems, then DFGVariableSummary (DataLevel1) came along and is exposing two competing needs which are being wrestled out as softtypename.

Requirements

• ("Manifest" aspect) information like which Manifolds, variable dimension, or Visualization projection functions to use.
• (Stateful aspect) When a user suddenly has some funky situation that requires additional variable information that must be available during deep-inner-computation-loop computations.
  • Related, see FactorMetadata that exposes the softtype and other information to the solver.
  • Possible alternative, Smalldata is an ongoing discussion for which the design is still in flux, and might induce serious performance problems owing to risk of user adding Unstable Types into the deep-inner-computation-loops.
  • Prior example is DynPose2 need to store microsecond time to allow velocity calculation, which was only added as full timestamp much later -- however, the situation can repeat with some other user data that cannot be designed in advance.

Related Issues

• Manifolds.jl Standardization:
  • Integrate with Manifolds.jl RoME.jl#244
  • Integrate with Manifolds.jl ApproxManifoldProducts.jl#32
• DataLevel1 softtypename: was softypename::Symbol a bad idea? DistributedFactorGraphs.jl#237
• smalldata:
  • Serialize smalldata to a blob-less entry in CGDFG DistributedFactorGraphs.jl#509
  • Discussion: Pass variables to factor evaluation so that we can use variable data? #784
• Towards immutable FactorMetadata(...) #825

Plan of Action

Can split these into more issues once a design decision is made.

• quickly do change Singleton / Pairwise to AbstractPrior / AbstractRelativeFactor DistributedFactorGraphs.jl#533 as easy picking to solve naming problem,
• Decide whether softtype::T should have stateful information or where to offload that requirement?
  • DF vote +1 to split stateful aspect out, see mechanism vote hereafter.
  • SC +1 to split it out. If we're going to store info, I'd like it in the variables, even if there is a performance hit (factor evaluation may retrieve smallData, or even bigData - that's up to the user). The alternative (duplicating data) I think is arguably worse.
  • JT vote +1 to split out. split inference variables (currently described by the softtype) from "constants" (the utime field for example) - I want to keep the possibility open that softtype becomes a hard type of a manifold (it feels right, but its a whole new issue) and think this is a step in that direction.
• Decide how to solve future user stateful variable data for computations problem
  • See proposals here: Manifest VariableTypes (Singleton model) and new FMD #783 (comment)
    • DF vote +1: use "inheritance"/"interface"/"traits" from a singleton type like Pose2 that you "inherit" and add own data to
    • SC +1 to just exposing the data to the user and letting them decide (i.e. you can make your own factor types, but you can also retrieve variable data).
    • JT I'll need a bit more insight into the uses and core inference before I can say. Currently, I'm leaning towards: It should be in solver data (lv2). Singleton types for now.
  • requires solution for ensuring type stability and can implement that process in Discussion: Pass variables to factor evaluation so that we can use variable data? #784 once decision on mechanism is made in this issue 783.
• Decide how getSofttype should work.
  • DF vote+1 for anything that follows from above votes.
  • SC +1 to it's stateless Singleton
  • JT +1 stateless - for now Singleton.
• Integrate new stateful mechanism with FactorMetadata
  • Likely will also be part of Discussion: Pass variables to factor evaluation so that we can use variable data? #784
• Documentation on how a user should add computational data and use in computations
  • e.g. first use see Changing radar factor to use a correct serialization strategy Caesar.jl#611
• Deprecate name softtype to only VariableType see Deprecation, rename softtype -> VariableType DistributedFactorGraphs.jl#603

---

Original Question

We need to fix all softtypes to have a default constructors, and take out all state information.

Types with issues:

struct ContinuousMultivariate{T1 <: Tuple} <: InferenceVariable
  dims::Int
  manifolds::T1
  ContinuousMultivariate{T}() where {T} = new()
  ContinuousMultivariate{T}(x::Int; manifolds::T=(:Euclid,)) where {T <: Tuple} = new(x, manifolds)
end

mutable struct DynPose2 <: IncrementalInference.InferenceVariable
  ut::Int64 # microsecond time
  dims::Int
  manifolds::Tuple{Symbol,Symbol,Symbol,Symbol,Symbol}
  DynPose2(;ut::Int64=-9999999999) = new(ut, 5, (:Euclid,:Euclid,:Circular,:Euclid,:Euclid))
end

struct ContinuousScalar <: InferenceVariable
  dims::Int
  manifolds::Tuple{Symbol}
  ContinuousScalar(;manifolds::Tuple{Symbol}=(:Euclid,)) = new(1, manifolds)
end

DF EDIT, a related issue is how to make it easy for the user to add their own data types to the default factors. Softtype with state solved that problem, but making it a manifest or hardtype removes that option.
SC: I suggest we add smallData to factors.

[Affie]

Just for reference: https://docs.julialang.org/en/v1/manual/types/#man-singleton-types-1

[Affie]

Singleton types can work with current Pose2 for example:

struct Pose2 <: IncrementalInference.InferenceVariable
  dims::Int
  manifolds::Tuple{Symbol,Symbol,Symbol}
  Pose2() = new(3, (:Euclid, :Euclid, :Circular))
end

as

struct Pose2 end
getdims(::Pose2) = 3
getmanifolds(::Pose2) = (:Euclid, :Euclid, :Circular)

[dehann]

Also see JuliaRobotics/RoME.jl#244 on standardizing with Manifolds.jl.

[dehann]

Copy from Slack, @GearsAD asked if this was workable:

# TODO: Confirm that we can switch this out, instead of retrieving the complete variable.
# Original getSofttype(getVariable(dfg,lbl), solvekey)
getSofttype(v::DFGVariable) = typeof(v).parameters[1]()

[Affie]

FIY, I don't know if the introspection is the best way in: getSofttype(v::DFGVariable) = typeof(v).parameters[1]()
rather use: getSofttype(::DFGVariable{T}) where T = T()

Edit: added the brackets, the 2 functions are equivalent. This is just to show a cleaner way to write the same function.

---

DF edit -- agree it's cleaner, sorry I misunderstood how T was being used.
For reference:
https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/blob/cdcc0dd0a77fc22f163aefb61db3b105a09746cf/src/entities/DFGVariable.jl#L28

[Affie]

I wanted the same concept in the summary graphs and basically did it with a warning:
https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/blob/6b903b79b15c6754191cfb2b673dd89d1eb97e2c/src/services/DFGVariable.jl#L293-L296

also see: JuliaRobotics/DistributedFactorGraphs.jl#237

[dehann]

rather use dispatch: getSofttype(::DFGVariable{T}) where T = T

that would be a hard type -- that's an even bigger discussion. Going softtype allows a bit more freedom and does not require enumeration of all the factor permutations.

EDIT
see edit above: #783 (comment)

a related issue is how to make it easy for the user to add their own data types to the default factors. Softtype with state solved that problem, but making it a manifest or hardtype removes that option.

[Affie]

This issue is closely related to the possible future use of a 'manifold type'. I don't quite understand the whole hard-type soft-type thing yet, but that's beside the point. I can see how ContinuousMultivariate can maybe be referred to as a soft-type. The other types are closely related to traits of the value array (it tells the algorithm how to handle the values).

• ContinuousMultivariate: as Sam suggested, it can be solved by having users just define one for their application. ContinuousMultivariate4 or ContinuousMultivariate5, for example.

  • DF edit, not sure how I feel about that yet, but since this is not used so much yet perhaps a better solution will become apparent as we solve the split data and manifest decision.

• Additional fields: I would guess the possible use of a manifold type would also require a split in the data with something like the field ut::Int64 a bit of a grey area. This would probably require another structure specifically for this kind of data. like metasolverdata

  • DF edit, we should just clear about the differences between "smalldata" and "metadata", I added vote to your suggestion below.

So is the core of the issue:

• How do we add user data to VariableNodeData to be used in the solver?
• Do we split out the user solver data?

---

Split Data Proposals

some options off the top of my head:

• Use "smalldata"
  • slow
  • not available
• use "softtype"
• use "user-meta-solver-data" field (probably quite similar to the factor meta data field?) + Singleton types (or later manifold)
• use "inheritance"/"interface"/"traits", where you have a singleton type like Pose2 that you "inherit" and add your own data to.
  • DF edit, This sounds like the most intriguing option to me -- will require blip of documentation to make it accessible. That way if the whole thing slows down badly during inference, it's likely the user not following type-stability principles. This way we don't put too much burden on the user or the developer. I like.
• other...

In the visualization use case, I needed the type to visualize it. There are only so many options that can currently be visualised. I only wanted Gaussian with a point, so PPE, but its useless without the softtype to tell you what is going on. With "summary graph", softtypename was enough for the basic types such as Pose2 and Point2 (The singleton types). The rest is user-specific and hard (impossible) to make a generic visualiser for.

[Affie]

that would be a hard type -- that's an even bigger discussion. Going softtype allows a bit more freedom and does not require enumeration of all the factor permutations.

See edit above in the comment; to make reason a bit clearer and fix brackets.

@dehann, I don't know if I understand your comment correctly, maybe elaborate if it's still applicable after the edit?

[Affie]

@GearsAD if there are not too many, perhaps this idea could work for the short term. The union is not the best and just to illustrate the idea.

getSofttype(::DFGVariable{T}) where T <:  Union{"all the singleton types"} = T()
getSofttype(::DFGVariable{T}) where T <:  Union{"all the rest"} = ...as it was before...

[dehann]

Hi, trying to simplify and keep short issue -- see summary in first comment edit: #783 (comment)

I don't know if I understand your comment correctly, maybe elaborate if it's still applicable after the edit?

If I understood you right, here is my comment update: #783 (comment)

How do we add user data to VariableNodeData to be used in the solver?
Do we split out the user solver data?

I added my vote in first comment, and edited the Johan's suggestion above to add my support for which way forward.

[GearsAD]

Singleton types can work with current Pose2 for example:

struct Pose2 <: IncrementalInference.InferenceVariable
  dims::Int
  manifolds::Tuple{Symbol,Symbol,Symbol}
  Pose2() = new(3, (:Euclid, :Euclid, :Circular))
end

as

struct Pose2 end
getdims(::Pose2) = 3
getmanifolds(::Pose2) = (:Euclid, :Euclid, :Circular)

Right, so this is a more explicit version of the same idea, although in this case we'd be using the singleton form of Pose2? That sounds like a reasonable way to enforce that it doesn't retain state.

[GearsAD]

As in the edits in the initial description:

• I think we should provide the variables+factor in the factor evaluation for flexibility. If a factor requires variable data (or data from it's own factor) then it may be slow (is it really though?), but it may be necessary.
• I think the Singleton model from @Affie's suggestion would enforce the stateless structure, I'm just thinking we may want to serialize and save it somewhere. If we go with that, it's hardcoded so we can't do that.

[dehann]

it may be slow (is it really though?)

if we use the inheritance approach Johan suggests then we force the user to keep type safety (if I'm understanding the use right). I was trying to do the same thing by using softtype originally, but now we need to do more things with it -- so this is a feature expansion process actually.

type stability is a major performance thing, so we do need a reasonable approach to keeping that for normal use. An intermediate user should be able to augment their own data (like the radar images) and still have type safety. I think if a new user is just trying something, then combo of slow performance and documentation should help them advance towards an intermediate user (i.e. using user type for cached data).

I'm just thinking we may want to serialize and save it somewhere

That goes back to smalldata. How much should we force type stability in small data?

[GearsAD]

Currently we're proposing smalldata is like Union{Int, Float64, String, Vector{Int}, Vector{Float64, Vector{String}}. I think that should be a reasonable amount of flexibility and won't hammer performance?

[dehann]

Oh, that will hammer performance unfortunately, the object containing a field ::Union{...} is the potential problem there (85% sure about that). So either we should think of a way to harden smalldata, or as Johan suggests introduce yet another field "metadata". I'm not too hot on the additional field thing, personally I'd rather just limit the user options on smalldata. However, I think @Affie has something clever up his sleeve to use inheritance with "traits" to build 3rd option here that does a good enough job on both concerns (type safety and flexibility, but usage is one up from novice).

EDIT: functions dispatching on a parameter ::Union{..} or ::Abstract is fine, but challenge comes in with ::Union or ::Abstract type fields -- the latter uses "dynamic dispatch" to interpret on the fly how to work with the data. The earlier case of dispatch on functions can preemptively do type-inference and then statically compile a hardtype function.

[Affie]

Currently we're proposing smalldata is like Union{Int, Float64, String, Vector{Int}, Vector{Float64, Vector{String}}. I think that should be a reasonable amount of flexibility and won't hammer performance?

Slightly off topic:

This will perform way better than a "type stable" string that has to be parsed every time.

If you limit the instability with other techniques such as adding type annotations and/or breaking up functions, it may be a good trade-off.

For example:

julia> d = Dict{Symbol, Any}();
julia> push!(d, :a=>5);
julia> push!(d, :b=>5.0)
Dict{Symbol,Any} with 2 entries:
  :a => 5
  :b => 5.0

julia> function sumD(d)
           return d[:a]::Int + d[:b]::Float64
       end
sumD (generic function with 1 method)

julia> @code_warntype sumD(d)
Variables
  #self#::Core.Compiler.Const(sumD, false)
  d::Dict{Symbol,Any}

Body::Float64
1 ─ %1 = Base.getindex(d, :a)::Any
│   %2 = Core.typeassert(%1, Main.Int)::Int64
│   %3 = Base.getindex(d, :b)::Any
│   %4 = Core.typeassert(%3, Main.Float64)::Float64
│   %5 = (%2 + %4)::Float64
└──      return %5

[Affie]

Right, so this is a more explicit version of the same idea, although in this case we'd be using the singleton form of Pose2? That sounds like a reasonable way to enforce that it doesn't retain state.

The reason to rather use a singleton is it can only have one instance. eg

julia> Pose2() === Pose2()
true

I think the Singleton model from @Affie's suggestion would enforce the stateless structure, I'm just thinking we may want to serialize and save it somewhere. If we go with that, it's hardcoded so we can't do that.

Currently, Pose2 requires RoME to use, so I don't think it would make a difference. It makes it easier as you can just serialize the name, ie "Pose2" and not any fields. So getSofttype(::DFGVariable{T}) where T = T() will still work.

[Affie]

Added advantage, micro performance improvement. It might be negligible but this way the constant propagation can be used:

#the 2 versions of:
@code_lowered getDims(p3)
# old
CodeInfo(
1 ─ %1 = Base.getproperty(p, :dims)
└──      return %1
)
# new
CodeInfo(
1 ─     return 6
)

[dehann]

nice!

[dehann]

Decision for #784 made, consolidating work effort in this issue.

[Affie]

Partially addressed by #823 - with full variables.
Deprecation stared to change softtypes to singletons

[dehann]

lol JuliaRobotics/RoME.jl#62

[dehann]

see JuliaRobotics/DistributedFactorGraphs.jl#603

[Affie]

Stared the deprecation process in #841.

[dehann]

Too big a change to also include in v0.17.0, punting down one to v0.18.0 since this will require a breaking change upstream from DFG v0.11.0 which is not available yet.

[dehann]

xref #825