VariableState label required and more VariableState related cleanup (#1155)

Author: Affie

src/Deprecated.jl

@@ -20,6 +20,37 @@ const InferenceType = AbstractPackedFactorObservation
 
 const PackedSamplableBelief = PackedBelief
 
+export setSolverData!
+"""
+    $SIGNATURES
+Set solver data structure stored in a variable.
+"""
+function setSolverData!(v::VariableCompute, data::VariableState, key::Symbol = :default)
+    Base.depwarn(
+        "setSolverData!(v::VariableCompute, data::VariableState, key::Symbol = :default) is deprecated, use mergeVariableState! instead.",
+        :setSolverData!,
+    )
+    @assert key == data.solveKey "VariableState.solveKey=:$(data.solveKey) does not match requested :$(key)"
+    return v.solverDataDict[key] = data
+end
+
+@deprecate mergeVariableSolverData!(args...; kwargs...) mergeVariableState!(
+    args...;
+    kwargs...,
+)
+
+export mergeVariableData!, mergeGraphVariableData!
+function mergeVariableData!(args...)
+    return error(
+        "mergeVariableData! is obsolete, use mergeVariableState! for state, PPEs are obsolete",
+    )
+end
+function mergeGraphVariableData!(args...)
+    return error(
+        "mergeGraphVariableData! is obsolete, use mergeVariableState! for state, PPEs are obsolete",
+    )
+end
+
 ## ================================================================================
 ## Deprecated in v0.27
 ##=================================================================================

src/DistributedFactorGraphs.jl

@@ -58,9 +58,9 @@ using InteractiveUtils: subtypes
 ##==============================================================================
 
 # v1 name, signiture, return, and error checked
+export getFactor, getBlobentry, getGraphBlobentry
 
 # v1 name, signiture, and return
-export getFactor, getBlobentry, getGraphBlobentry, getVariableState, getFactorState
 
 # v1 name only
 export getVariable, getBlob, addBlob!
@@ -71,6 +71,9 @@ export DFG
 
 export GraphsDFGs, GraphsDFG
 
+##
+export getVariableState, getFactorState # FIXME these were questioned and being reviewed again for name, other than that they are checked.
+
 ##------------------------------------------------------------------------------
 ## DFG
 ##------------------------------------------------------------------------------
@@ -174,7 +177,7 @@ export listTags, mergeTags!, removeTags!, emptyTags!
 export VariableStateType
 
 # accessors
-export getSolverDataDict, setSolverData!
+export getSolverDataDict
 export getVariableType, getVariableTypeName
 
 export getObservation
@@ -201,7 +204,6 @@ export getVariableStates,
     mergeVariableState!,
     deleteVariableState!,
     listVariableStates,
-    mergeVariableSolverData!,
     cloneSolveKey!
 
 # PPE
@@ -264,9 +266,6 @@ export FactorSolverCache
 export getVariableOrder
 export getFactorType, getFactorFunction
 
-# Node Data
-export mergeVariableData!, mergeGraphVariableData!
-
 # Serialization type conversion
 export convertPackedType, convertStructType
 

src/GraphsDFG/services/GraphsDFG.jl

@@ -512,6 +512,9 @@ end
 
 # FG blob entries 
 function getGraphBlobentry(fg::GraphsDFG, label::Symbol)
+    if !haskey(fg.graphBlobEntries, label)
+        throw(LabelNotFoundError("GraphBlobentry", label))
+    end
     return fg.graphBlobEntries[label]
 end
 

src/services/AbstractDFG.jl

@@ -1418,52 +1418,6 @@ function mergeGraph!(
     return destDFG
 end
 
-##==============================================================================
-## Variable Data: VND and PPE
-##==============================================================================
-
-#TODO API
-"""
-    $(SIGNATURES)
-Merges and updates solver and estimate data for a variable (variable can be from another graph).
-Note: Makes a copy of the estimates and solver data so that there is no coupling between graphs.
-"""
-function mergeVariableData!(dfg::AbstractDFG, sourceVariable::AbstractDFGVariable)
-    var = getVariable(dfg, sourceVariable.label)
-
-    mergePPEs!(var, sourceVariable)
-    # If this variable has solverDataDict (summaries do not)
-    :solverDataDict in fieldnames(typeof(var)) &&
-        mergeVariableSolverData!(var, sourceVariable)
-
-    #update if its not a InMemoryDFGTypes, otherwise it was a reference
-    # if satelite nodes are used it can be updated separately
-    # !(isa(dfg, InMemoryDFGTypes)) && mergeVariable!(dfg, var)
-
-    return var
-end
-
-#TODO API
-"""
-    $(SIGNATURES)
-Common function to update all solver data and estimates from one graph to another.
-This should be used to push local solve data back into a cloud graph, for example.
-
-Notes
-- Returns `::Nothing`
-"""
-function mergeGraphVariableData!(
-    destDFG::H,
-    sourceDFG::G,
-    varSyms::Vector{Symbol},
-) where {G <: AbstractDFG, H <: AbstractDFG}
-    # Update all variables in the destination
-    # (For now... we may change this soon)
-    for variableId in varSyms
-        mergeVariableData!(destDFG, getVariable(sourceDFG, variableId))
-    end
-end
-
 ##==============================================================================
 ## Graphs Structures (Abstract, overwrite for performance)
 ##==============================================================================

src/services/CompareUtils.jl

@@ -253,8 +253,8 @@ function compareVariable(
     union!(skiplist, skip)
     TP = TP && compareAll(A.solverDataDict, B.solverDataDict; skip = skiplist, show = show)
 
-    Ad = getVariableState(A)
-    Bd = getVariableState(B)
+    Ad = getVariableState(A, :default) #FIXME why onlly comparing default?
+    Bd = getVariableState(B, :default)
 
     # TP = TP && compareAll(A.attributes, B.attributes, skip=[:variableType;], show=show)
     varskiplist = union(varskiplist, [:variableType])

src/services/DFGVariable.jl

@@ -538,26 +538,15 @@ getSolverDataDict(v::VariableCompute) = v.solverDataDict
 
 Retrieve solver data structure stored in a variable.
 """
-function getVariableState(v::VariableCompute, key::Symbol = :default)
-    #TODO this does not fit in with some of the other error behaviour. but its used so added @error
-    vnd = if haskey(getSolverDataDict(v), key)
-        return getSolverDataDict(v)[key]
+function getVariableState(v::VariableCompute, label::Symbol)
+    vnd = if haskey(getSolverDataDict(v), label)
+        return getSolverDataDict(v)[label]
     else
-        throw(LabelNotFoundError("State", key))
+        throw(LabelNotFoundError("State", label))
     end
     return vnd
 end
 
-#TODO Repeated functionality? same as update
-"""
-    $SIGNATURES
-Set solver data structure stored in a variable.
-"""
-function setSolverData!(v::VariableCompute, data::VariableState, key::Symbol = :default)
-    @assert key == data.solveKey "VariableState.solveKey=:$(data.solveKey) does not match requested :$(key)"
-    return v.solverDataDict[key] = data
-end
-
 ##------------------------------------------------------------------------------
 ## Variable Metadata
 ##------------------------------------------------------------------------------
@@ -682,49 +671,36 @@ end
     $(SIGNATURES)
 Get variable solverdata for a given solve key.
 """
-function getVariableState(
-    dfg::AbstractDFG,
-    variablekey::Symbol,
-    solvekey::Symbol = :default,
-)
-    v = getVariable(dfg, variablekey)
-    !haskey(v.solverDataDict, solvekey) && throw(LabelNotFoundError("State", solvekey))
-    return v.solverDataDict[solvekey]
+function getVariableState(dfg::AbstractDFG, variableLabel::Symbol, label::Symbol)
+    v = getVariable(dfg, variableLabel)
+    !haskey(v.solverDataDict, label) && throw(LabelNotFoundError("State", label))
+    return v.solverDataDict[label]
 end
 
-function getVariableStates(dfg::AbstractDFG, variablekey::Symbol)
-    v = getVariable(dfg, variablekey)
+function getVariableStates(dfg::AbstractDFG, variableLabel::Symbol)
+    v = getVariable(dfg, variableLabel)
     return collect(values(v.solverDataDict))
 end
 
 """
     $(SIGNATURES)
 Add variable solver data, errors if it already exists.
 """
-function addVariableState!(dfg::AbstractDFG, variablekey::Symbol, vnd::VariableState)
+function addVariableState!(dfg::AbstractDFG, variablekey::Symbol, state::VariableState)
     var = getVariable(dfg, variablekey)
-    if haskey(var.solverDataDict, vnd.solveKey)
-        throw(LabelExistsError("VariableState", vnd.solveKey))
+    if haskey(var.solverDataDict, state.solveKey)
+        throw(LabelExistsError("VariableState", state.solveKey))
     end
-    var.solverDataDict[vnd.solveKey] = vnd
-    return vnd
+    var.solverDataDict[state.solveKey] = state
+    return state
 end
 
-"""
-    $(SIGNATURES)
-Add a new solver data  entry from a deepcopy of the source variable solver data.
-NOTE: Copies the solver data.
-"""
-function addVariableState!(
-    dfg::AbstractDFG,
-    sourceVariable::VariableCompute,
-    solveKey::Symbol = :default,
-)
-    return addVariableState!(
-        dfg,
-        sourceVariable.label,
-        deepcopy(getVariableState(sourceVariable, solveKey)),
-    )
+function addVariableState!(v, state::VariableState)
+    if haskey(v.solverDataDict, state.solveKey)
+        throw(LabelExistsError("VariableState", state.solveKey))
+    end
+    v.solverDataDict[state.solveKey] = state
+    return state
 end
 
 """
@@ -747,6 +723,16 @@ function mergeVariableState!(dfg::AbstractDFG, variablekey::Symbol, vnd::Variabl
     return 1
 end
 
+function mergeVariableState!(v::VariableCompute, vnd::VariableState)
+    if !haskey(v.solverDataDict, vnd.solveKey)
+        addVariableState!(v, vnd)
+    else
+        v.solverDataDict[vnd.solveKey] = vnd
+    end
+
+    return 1
+end
+
 function copytoVariableState!(
     dfg::AbstractDFG,
     variableLabel::Symbol,
@@ -797,13 +783,9 @@ end
 
 """
     $(SIGNATURES)
-Delete variable solver data, returns the deleted element.
+Delete variable solver data, returns the number of deleted elements.
 """
-function deleteVariableState!(
-    dfg::AbstractDFG,
-    variablekey::Symbol,
-    solveKey::Symbol = :default,
-)
+function deleteVariableState!(dfg::AbstractDFG, variablekey::Symbol, solveKey::Symbol)
     var = getVariable(dfg, variablekey)
 
     if !haskey(var.solverDataDict, solveKey)
@@ -815,12 +797,12 @@ end
 
 """
     $(SIGNATURES)
-Delete variable solver data, returns the deleted element.
+Delete variable solver data, returns the number of deleted elements.
 """
 function deleteVariableState!(
     dfg::AbstractDFG,
     sourceVariable::VariableCompute,
-    solveKey::Symbol = :default,
+    solveKey::Symbol,
 )
     return deleteVariableState!(dfg, sourceVariable.label, solveKey)
 end
@@ -838,28 +820,6 @@ function listVariableStates(dfg::AbstractDFG, variablekey::Symbol)
     return collect(keys(v.solverDataDict))
 end
 
-"""
-    $(SIGNATURES)
-Merges and updates solver and estimate data for a variable (variable can be from another graph).
-If the same key is present in another collection, the value for that key will be the value it has in the last collection listed (updated).
-Note: Makes a copy of the estimates and solver data so that there is no coupling between graphs.
-"""
-function mergeVariableSolverData!(
-    destVariable::VariableCompute,
-    sourceVariable::VariableCompute,
-)
-    # We don't know which graph this came from, must be copied!
-    merge!(destVariable.solverDataDict, deepcopy(sourceVariable.solverDataDict))
-    return destVariable
-end
-
-function mergeVariableSolverData!(dfg::AbstractDFG, sourceVariable::VariableCompute)
-    return mergeVariableSolverData!(
-        getVariable(dfg, getLabel(sourceVariable)),
-        sourceVariable,
-    )
-end
-
 ##==============================================================================
 ## Point Parametric Estimates
 ##==============================================================================

test/fileDFGTests.jl

@@ -19,7 +19,10 @@ using UUIDs
             1:numNodes,
         )
         map(v -> setSolvable!(v, Int(round(rand()))), verts)
-        map(v -> getVariableState(verts[4]).solveInProgress = Int(round(rand())), verts)
+        map(
+            v -> getVariableState(verts[4], :default).solveInProgress = Int(round(rand())),
+            verts,
+        )
         map(v -> setSolvedCount!(v, Int(round(10 * rand()))), verts)
 
         # Add some data entries