Deprecate Variable[NodeData/SolverData] to VariableState (#1139)

Author: Affie

NEWS.md

@@ -12,6 +12,13 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
     - This is used internally be the solver and should not affect the average user of DFG.
     - Rename FactorOperationalMemory -> FactorSolverCache
     - Rename AbstractFactor -> AbstractFactorObservation (keeping both around)
+- Deprecate VariableNodeData -> VariableState
+- Deprecate getVariableSolverData -> getVariableState
+- Deprecate addVariableSolverData! -> addVariableState!
+- Deprecate deleteVariableSolverData! -> deleteVariableState!
+- Deprecate listVariableSolverData -> listVariableStates
+- Deprecate getVariableSolverDataAll -> getVariableStates
+- Deprecate getSolverData -> getVariableState/getFactorState
 
 # v0.26
 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`.

docs/src/GraphData.md

@@ -15,7 +15,7 @@ The following is a guideline to using these parameters.
 
 **NOTE**: Adds in general throw an error if the element already exists. Update will update the element if it exists, otherwise it will add it.
 
-**NOTE**: In general these functions will return an error if the respective element is not found. This is to avoid returning, say, nothing, which will be horribly confusing if you tried `getVariableSolverData(dfg, :a, :b)` and it returned nothing - which was missing, :a or :b, or was there a communication issue? We recommend coding defensively and trapping errors in critical portions of your user code.
+**NOTE**: In general these functions will return an error if the respective element is not found. This is to avoid returning, say, nothing, which will be horribly confusing if you tried `getVariableState(dfg, :a, :b)` and it returned nothing - which was missing, :a or :b, or was there a communication issue? We recommend coding defensively and trapping errors in critical portions of your user code.
 
 **NOTE**: All data is passed by reference, so if you update the returned structure it will update in the graph. The database driver is an exception, and once the variable or factor is updated you need to call update* to persist the changes to the graph.
 
@@ -125,26 +125,26 @@ Solver data is used by IncrementalInference/RoME/Caesar solver to produce the ab
 Related functions:
 
 
-- [`listVariableSolverData`](@ref)
-- [`getVariableSolverData`](@ref)
-- [`addVariableSolverData!`](@ref)
+- [`listVariableStates`](@ref)
+- [`getVariableState`](@ref)
+- [`addVariableState!`](@ref)
+- [`mergeVariableState!`](@ref)
+- [`deleteVariableState!`](@ref)
 - [`mergeVariableState!`](@ref)
-- [`deleteVariableSolverData!`](@ref)
-- [`mergeVariableSolverData!`](@ref)
 
 
 Example of solver data operations:
 
 ```julia
 # Add new VND of type ContinuousScalar to :x0
-# Could also do VariableNodeData(ContinuousScalar())
-vnd = VariableNodeData{ContinuousScalar}()
-addVariableSolverData!(dfg, :x0, vnd, :parametric)
-@show listVariableSolverData(dfg, :x0)
+# Could also do VariableState(ContinuousScalar())
+vnd = VariableState{ContinuousScalar}()
+addVariableState!(dfg, :x0, vnd, :parametric)
+@show listVariableStates(dfg, :x0)
 # Get the data back - note that this is a reference to above.
-vndBack = getVariableSolverData(dfg, :x0, :parametric)
+vndBack = getVariableState(dfg, :x0, :parametric)
 # Delete it
-deleteVariableSolverData!(dfg, :x0, :parametric)
+deleteVariableState!(dfg, :x0, :parametric)
 ```
 
 #### Small Data

src/Deprecated.jl

@@ -10,6 +10,9 @@ const AbstractPackedFactor = AbstractPackedFactorObservation
 export FactorOperationalMemory
 const FactorOperationalMemory = FactorSolverCache
 
+export VariableNodeData
+const VariableNodeData = VariableState
+
 @deprecate getNeighborhood(args...; kwargs...) listNeighborhood(args...; kwargs...)
 @deprecate addBlob!(store::AbstractBlobstore, blobId::UUID, data, ::String) addBlob!(
     store,
@@ -70,13 +73,30 @@ const FactorOperationalMemory = FactorSolverCache
 )
 @deprecate mergeBlobEntries!(args...; kwargs...) mergeBlobentries!(args...; kwargs...)
 
+@deprecate getVariableSolverData(args...; kwargs...) getVariableState(args...; kwargs...)
+@deprecate addVariableSolverData!(args...; kwargs...) addVariableState!(args...; kwargs...)
+@deprecate deleteVariableSolverData!(args...; kwargs...) deleteVariableState!(
+    args...;
+    kwargs...,
+)
+@deprecate listVariableSolverData(args...; kwargs...) listVariableStates(args...; kwargs...)
+@deprecate getVariableSolverDataAll(args...; kwargs...) getVariableStates(
+    args...;
+    kwargs...,
+)
+
+@deprecate getSolverData(v::VariableCompute, solveKey::Symbol = :default) getVariableState(
+    v,
+    solveKey,
+)
+
 export updateVariableSolverData!
 
 #TODO possibly completely deprecated or not exported until update verb is standardized
 function updateVariableSolverData!(
     dfg::AbstractDFG,
     variablekey::Symbol,
-    vnd::VariableNodeData,
+    vnd::VariableState,
     useCopy::Bool = false,
     fields::Vector{Symbol} = Symbol[];
     warn_if_absent::Bool = true,
@@ -89,14 +109,14 @@ function updateVariableSolverData!(
     var = getVariable(dfg, variablekey)
     warn_if_absent &&
         !haskey(var.solverDataDict, vnd.solveKey) &&
-        @warn "VariableNodeData '$(vnd.solveKey)' does not exist, adding"
+        @warn "VariableState '$(vnd.solveKey)' does not exist, adding"
 
     # for InMemoryDFGTypes do memory copy or repointing, for cloud this would be an different kind of update.
     usevnd = vnd # useCopy ? deepcopy(vnd) : vnd
     # should just one, or many pointers be updated?
     useExisting =
         haskey(var.solverDataDict, vnd.solveKey) &&
-        isa(var.solverDataDict[vnd.solveKey], VariableNodeData) &&
+        isa(var.solverDataDict[vnd.solveKey], VariableState) &&
         length(fields) != 0
     # @error useExisting vnd.solveKey
     if useExisting
@@ -123,17 +143,17 @@ end
 function updateVariableSolverData!(
     dfg::AbstractDFG,
     variablekey::Symbol,
-    vnd::VariableNodeData,
+    vnd::VariableState,
     solveKey::Symbol,
     useCopy::Bool = false,
     fields::Vector{Symbol} = Symbol[];
     warn_if_absent::Bool = true,
 )
     # TODO not very clean
     if vnd.solveKey != solveKey
-        @warn(
-            "updateVariableSolverData with solveKey parameter might change in the future, see DFG #565. Future warnings are suppressed",
-            maxlog = 1
+        Base.depwarn(
+            "updateVariableSolverData with solveKey is deprecated use copytoVariableState! instead.",
+            :updateVariableSolverData!,
         )
         usevnd = useCopy ? deepcopy(vnd) : vnd
         usevnd.solveKey = solveKey
@@ -170,7 +190,7 @@ function updateVariableSolverData!(
     # toshow = listSolveKeys(sourceVariable) |> collect
     # @info "update DFGVar solveKey" solveKey vnd.solveKey 
     # @show toshow
-    @assert solveKey == vnd.solveKey "VariableNodeData's solveKey=:$(vnd.solveKey) does not match requested :$solveKey"
+    @assert solveKey == vnd.solveKey "VariableState's solveKey=:$(vnd.solveKey) does not match requested :$solveKey"
     return updateVariableSolverData!(
         dfg,
         sourceVariable.label,
@@ -258,7 +278,7 @@ export getSolverData, setSolverData!
 
 function getSolverData(f::FactorCompute)
     return error(
-        "getSolverData(f::FactorCompute) is obsolete, use getState, getObservation, or getCache instead",
+        "getSolverData(f::FactorCompute) is obsolete, use getFactorState, getObservation, or getCache instead",
     )
 end
 

src/DistributedFactorGraphs.jl

@@ -167,7 +167,7 @@ export getSolverDataDict, setSolverData!
 export getVariableType, getVariableTypeName
 
 export getObservation
-export getState, getFactorState
+export getFactorState
 
 export getVariableType
 
@@ -186,11 +186,12 @@ export getMetadata,
     emptyMetadata!
 
 # CRUD & SET
-export getVariableSolverData,
-    addVariableSolverData!,
+export getVariableState,
+    getVariableStates,
+    addVariableState!,
     mergeVariableState!,
-    deleteVariableSolverData!,
-    listVariableSolverData,
+    deleteVariableState!,
+    listVariableStates,
     mergeVariableSolverData!,
     cloneSolveKey!
 
@@ -211,9 +212,9 @@ export getPPE,
 
 # Variable Node Data
 ##------------------------------------------------------------------------------
-export VariableNodeData, PackedVariableNodeData
+export VariableState, PackedVariableState
 
-export packVariableNodeData, unpackVariableNodeData
+export packVariableState, unpackVariableState
 
 export getSolvedCount,
     isSolved, setSolvedCount!, isInitialized, isMarginalized, setMarginalized!

src/entities/DFGFactor.jl

@@ -283,7 +283,7 @@ function Base.getproperty(x::FactorCompute, f::Symbol)
     elseif f == :solverData
         # TODO remove, deprecated in v0.27
         error(
-            "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getState` or `getCache` instead.",
+            "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getFactorState` or `getCache` instead.",
         )
     elseif f == :_variableOrderSymbols
         [getfield(x, f)...]

src/entities/DFGVariable.jl

@@ -5,7 +5,7 @@
 abstract type InferenceVariable end
 
 ##==============================================================================
-## VariableNodeData
+## VariableState
 ##==============================================================================
 
 """
@@ -19,7 +19,7 @@ N: Manifold dimension.
 Fields:
 $(TYPEDFIELDS)
 """
-Base.@kwdef mutable struct VariableNodeData{T <: InferenceVariable, P, N}
+Base.@kwdef mutable struct VariableState{T <: InferenceVariable, P, N}
     "DEPRECATED remove in DFG v0.22"
     variableType::T = T() #tricky deprecation, also change covar to using N and not variableType
     """
@@ -72,7 +72,7 @@ Base.@kwdef mutable struct VariableNodeData{T <: InferenceVariable, P, N}
     """
     solvedCount::Int = 0
     """
-    solveKey identifier associated with this VariableNodeData object.
+    solveKey identifier associated with this VariableState object.
     """
     solveKey::Symbol = :default
     """
@@ -84,26 +84,26 @@ end
 
 ##------------------------------------------------------------------------------
 ## Constructors
-function VariableNodeData{T}(; kwargs...) where {T <: InferenceVariable}
-    return VariableNodeData{T, getPointType(T), getDimension(T)}(; kwargs...)
+function VariableState{T}(; kwargs...) where {T <: InferenceVariable}
+    return VariableState{T, getPointType(T), getDimension(T)}(; kwargs...)
 end
-function VariableNodeData(variableType::InferenceVariable; kwargs...)
-    return VariableNodeData{typeof(variableType)}(; kwargs...)
+function VariableState(variableType::InferenceVariable; kwargs...)
+    return VariableState{typeof(variableType)}(; kwargs...)
 end
 
 ##==============================================================================
-## PackedVariableNodeData.jl
+## PackedVariableState.jl
 ##==============================================================================
 
 """
 $(TYPEDEF)
-Packed VariableNodeData structure for serializing DFGVariables.
+Packed VariableState structure for serializing DFGVariables.
 
   ---
 Fields:
 $(TYPEDFIELDS)
 """
-Base.@kwdef mutable struct PackedVariableNodeData
+Base.@kwdef mutable struct PackedVariableState
     id::Union{UUID, Nothing} # If it's blank it doesn't exist in the DB.
     vecval::Vector{Float64}
     dimval::Int
@@ -130,9 +130,9 @@ end
 # createdTimestamp::DateTime#!
 # lastUpdatedTimestamp::DateTime#!
 
-StructTypes.StructType(::Type{PackedVariableNodeData}) = StructTypes.UnorderedStruct()
-StructTypes.idproperty(::Type{PackedVariableNodeData}) = :id
-StructTypes.omitempties(::Type{PackedVariableNodeData}) = (:id,)
+StructTypes.StructType(::Type{PackedVariableState}) = StructTypes.UnorderedStruct()
+StructTypes.idproperty(::Type{PackedVariableState}) = :id
+StructTypes.omitempties(::Type{PackedVariableState}) = (:id,)
 
 ##==============================================================================
 ## PointParametricEst
@@ -228,7 +228,7 @@ Base.@kwdef struct VariableDFG <: AbstractDFGVariable
     _version::String = string(_getDFGVersion())
     metadata::String = "e30="
     solvable::Int = 1
-    solverData::Vector{PackedVariableNodeData} = PackedVariableNodeData[]
+    solverData::Vector{PackedVariableState} = PackedVariableState[]
 end
 # maybe add to variable
 # createdTimestamp::DateTime
@@ -304,9 +304,9 @@ Base.@kwdef struct VariableCompute{T <: InferenceVariable, P, N} <: AbstractDFGV
     ppeDict::Dict{Symbol, AbstractPointParametricEst} =
         Dict{Symbol, AbstractPointParametricEst}()
     """Dictionary of solver data. May be a subset of all solutions if a solver label was specified in the get call.
-    Accessors: [`addVariableSolverData!`](@ref), [`mergeVariableState!`](@ref), and [`deleteVariableSolverData!`](@ref)"""
-    solverDataDict::Dict{Symbol, VariableNodeData{T, P, N}} =
-        Dict{Symbol, VariableNodeData{T, P, N}}()
+    Accessors: [`addVariableState!`](@ref), [`mergeVariableState!`](@ref), and [`deleteVariableState!`](@ref)"""
+    solverDataDict::Dict{Symbol, VariableState{T, P, N}} =
+        Dict{Symbol, VariableState{T, P, N}}()
     """Dictionary of small data associated with this variable.
     Accessors: [`getMetadata`](@ref), [`setMetadata!`](@ref)"""
     smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}()
@@ -343,7 +343,7 @@ function VariableCompute(label::Symbol, variableType::InferenceVariable; kwargs.
     return VariableCompute(label, typeof(variableType); kwargs...)
 end
 
-function VariableCompute(label::Symbol, solverData::VariableNodeData; kwargs...)
+function VariableCompute(label::Symbol, solverData::VariableState; kwargs...)
     return VariableCompute(;
         label,
         solverDataDict = Dict(:default => solverData),

src/services/CommonAccessors.jl

@@ -165,7 +165,7 @@ function getSolveInProgress(
         end
     end
     # Factor
-    return getState(var).solveInProgress
+    return getFactorState(var).solveInProgress
 end
 
 #TODO missing set solveInProgress and graph level accessor

src/services/CompareUtils.jl

@@ -19,8 +19,8 @@ implement compare if needed.
 # Generate compares automatically for all in this union
 const GeneratedCompareUnion = Union{
     MeanMaxPPE,
-    VariableNodeData,
-    PackedVariableNodeData,
+    VariableState,
+    PackedVariableState,
     VariableCompute,
     VariableDFG,
     VariableSummary,
@@ -193,8 +193,8 @@ function compareAll(
     return true
 end
 
-#Compare VariableNodeData
-function compare(a::VariableNodeData, b::VariableNodeData)
+#Compare VariableState
+function compare(a::VariableState, b::VariableState)
     a.val != b.val && @debug("val is not equal") == nothing && return false
     a.bw != b.bw && @debug("bw is not equal") == nothing && return false
     a.BayesNetOutVertIDs != b.BayesNetOutVertIDs &&
@@ -253,8 +253,8 @@ function compareVariable(
     union!(skiplist, skip)
     TP = TP && compareAll(A.solverDataDict, B.solverDataDict; skip = skiplist, show = show)
 
-    Ad = getSolverData(A)
-    Bd = getSolverData(B)
+    Ad = getVariableState(A)
+    Bd = getVariableState(B)
 
     # TP = TP && compareAll(A.attributes, B.attributes, skip=[:variableType;], show=show)
     varskiplist = union(varskiplist, [:variableType])
@@ -298,8 +298,8 @@ function compareFactor(
     @debug "compareFactor 1/5" TP
     TP =
         TP & compareAll(
-            getState(A),
-            getState(B);
+            getFactorState(A),
+            getFactorState(B);
             skip = union([:fnc; :_gradients], skip),
             show = show,
         )