update -> merge for Variable and Factor

Author: Affie

NEWS.md

@@ -2,6 +2,8 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
 
 # v0.27
 - `delete` returns number of nodes deleted and no longer the object that was deleted.
+- Deprecate `updateVariable!` for `mergeVariable!`, note `merege` returns number of nodes updated/added.
+- Deprecate `updateFactor!` for `mergeFactor!`, note `merege` returns number of nodes updated/added.
 
 # v0.26
 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`.

docs/src/BuildingGraphs.md

@@ -135,8 +135,8 @@ independently using the functions as discussed in the update section below.
 
 Full variables and factors can be updated using the following functions:
 
-- [`updateVariable!`](@ref)
-- [`updateFactor!`](@ref)
+- [`mergeVariable!`](@ref)
+- [`mergeFactor!`](@ref)
 
 
 **NOTE**: Skeleton and summary variables are read-only. To perform updates you

src/Deprecated.jl

@@ -3,8 +3,19 @@
 ##=================================================================================
 
 @deprecate getNeighborhood(args...; kwargs...) listNeighborhood(args...; kwargs...)
-@deprecate addBlob!(store::AbstractBlobStore, blobId::UUID, data, ::String) addBlob!(store, blobId, data)
-@deprecate addBlob!(store::AbstractBlobStore{T}, data::T, ::String) where {T} addBlob!(store, uuid4(), data)
+@deprecate addBlob!(store::AbstractBlobStore, blobId::UUID, data, ::String) addBlob!(
+    store,
+    blobId,
+    data,
+)
+@deprecate addBlob!(store::AbstractBlobStore, data, ::String) addBlob!(
+    store,
+    uuid4(),
+    data,
+)
+
+@deprecate updateVariable!(args...) mergeVariable!(args...)
+@deprecate updateFactor!(args...) mergeFactor!(args...)
 
 ## ================================================================================
 ## Deprecated in v0.25
@@ -69,4 +80,3 @@ DFGSummary(args) = error("DFGSummary is deprecated")
 )
 
 @deprecate lsfWho(dfg::AbstractDFG, type::Symbol) lsf(dfg, getfield(Main, type))
-

src/DistributedFactorGraphs.jl

@@ -110,8 +110,8 @@ export exists,
     addFactors!,
     getVariable,
     getFactor,
-    updateVariable!,
-    updateFactor!,
+    mergeVariable!,
+    mergeFactor!,
     deleteVariable!,
     deleteFactor!,
     listVariables,
@@ -145,7 +145,7 @@ export getSolvable, setSolvable!, isSolvable
 export getVariableLabelNumber
 
 # accessors
-export getLabel, getTimestamp, setTimestamp, setTimestamp!, getTags, setTags!
+export getLabel, getTimestamp, setTimestamp, getTags, setTags!
 
 export getAgentLabel, getGraphLabel
 

src/GraphsDFG/GraphsDFG.jl

@@ -27,8 +27,8 @@ import ...DistributedFactorGraphs:
     exists,
     isVariable,
     isFactor,
-    updateVariable!,
-    updateFactor!,
+    mergeVariable!,
+    mergeFactor!,
     deleteVariable!,
     deleteFactor!,
     getVariables,

src/GraphsDFG/services/GraphsDFG.jl

@@ -134,37 +134,29 @@ function getFactor(dfg::GraphsDFG, label::Symbol)
     return dfg.g.factors[label]
 end
 
-function updateVariable!(
-    dfg::GraphsDFG,
-    variable::AbstractDFGVariable;
-    warn_if_absent::Bool = true,
-)
+function mergeVariable!(dfg::GraphsDFG, variable::AbstractDFGVariable)
     if !haskey(dfg.g.variables, variable.label)
-        warn_if_absent &&
-            @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding"
-        return addVariable!(dfg, variable)
+        addVariable!(dfg, variable)
+    else
+        dfg.g.variables[variable.label] = variable
     end
-    dfg.g.variables[variable.label] = variable
-    return variable
+    return 1
 end
 
-function updateFactor!(
-    dfg::GraphsDFG,
-    factor::AbstractDFGFactor;
-    warn_if_absent::Bool = true,
-)
+function mergeFactor!(dfg::GraphsDFG, factor::AbstractDFGFactor;)
     if !haskey(dfg.g.factors, factor.label)
-        warn_if_absent &&
-            @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding"
-        return addFactor!(dfg, factor)
-    end
-
-    # Confirm that we're not updating the neighbors
-    dfg.g.factors[factor.label]._variableOrderSymbols != factor._variableOrderSymbols &&
+        addFactor!(dfg, factor)
+    elseif dfg.g.factors[factor.label]._variableOrderSymbols != factor._variableOrderSymbols
+        #TODO should we allow merging the factor neighbors or error as before?
         error("Cannot update the factor, the neighbors are not the same.")
+        # We need to delete the factor if we are updating the neighbors
+        deleteFactor!(dfg, factor.label)
+        addFactor!(dfg, factor)
+    else
+        dfg.g.factors[factor.label] = factor
+    end
 
-    dfg.g.factors[factor.label] = factor
-    return factor
+    return 1
 end
 
 function deleteVariable!(dfg::GraphsDFG, label::Symbol)#::Tuple{AbstractDFGVariable, Vector{<:AbstractDFGFactor}}

src/services/AbstractDFG.jl

@@ -213,7 +213,7 @@ function updateGraphMetadata!(dfg::AbstractDFG, pair::Pair{Symbol, String})
     return push!(dfg.graphMetadata, pair)
 end
 
-function deleteAgentMetadata!(dfg::AbstractDFG, key::Symbol) 
+function deleteAgentMetadata!(dfg::AbstractDFG, key::Symbol)
     pop!(dfg.agent.metadata, key)
     return 1
 end
@@ -392,21 +392,21 @@ end
 
 """
     $(SIGNATURES)
-Update a complete VariableCompute in the DFG.
+Merge a Variable to the DFG. Overwrites the existing variable if it exists or adds it if it does not.
 """
-function updateVariable!(
+function mergeVariable!(
     dfg::G,
     variable::V,
 ) where {G <: AbstractDFG, V <: AbstractDFGVariable}
-    return error("updateVariable! not implemented for $(typeof(dfg))")
+    return error("mergeVariable! not implemented for $(typeof(dfg))")
 end
 
 """
     $(SIGNATURES)
-Update a complete FactorCompute in the DFG.
+Merge a Factor to the DFG. Overwrites the existing factor if it exists or adds it if it does not.
 """
-function updateFactor!(dfg::G, factor::F) where {G <: AbstractDFG, F <: AbstractDFGFactor}
-    return error("updateFactor! not implemented for $(typeof(dfg))")
+function mergeFactor!(dfg::G, factor::F) where {G <: AbstractDFG, F <: AbstractDFGFactor}
+    return error("mergeFactor! not implemented for $(typeof(dfg))")
 end
 
 """
@@ -1097,7 +1097,7 @@ function copyGraph!(
         if !exists(destDFG, variable)
             addVariable!(destDFG, variableCopy)
         elseif overwriteDest
-            updateVariable!(destDFG, variableCopy)
+            mergeVariable!(destDFG, variableCopy)
         else
             error("Variable $(variable.label) already exists in destination graph!")
         end
@@ -1119,7 +1119,7 @@ function copyGraph!(
             if !exists(destDFG, factor)
                 addFactor!(destDFG, factorCopy)
             elseif overwriteDest
-                updateFactor!(destDFG, factorCopy)
+                mergeFactor!(destDFG, factorCopy)
             else
                 error("Factor $(factor.label) already exists in destination graph!")
             end
@@ -1458,7 +1458,7 @@ function mergeVariableData!(dfg::AbstractDFG, sourceVariable::AbstractDFGVariabl
 
     #update if its not a InMemoryDFGTypes, otherwise it was a reference
     # if satelite nodes are used it can be updated separately
-    # !(isa(dfg, InMemoryDFGTypes)) && updateVariable!(dfg, var)
+    # !(isa(dfg, InMemoryDFGTypes)) && mergeVariable!(dfg, var)
 
     return var
 end

src/services/CommonAccessors.jl

@@ -58,17 +58,14 @@ getTimestamp(v::DataLevel1) = v.timestamp
 
 Set the timestamp of a Variable/Factor object in a factor graph.
 Note:
-Since `timestamp` is not mutable `setTimestamp!` calls `updateVariable!` internally.
+Since `timestamp` is not mutable `setTimestamp!` calls `mergeVariable!` internally.
 See also [`setTimestamp`](@ref)
 """
 function setTimestamp!(dfg::AbstractDFG, lbl::Symbol, ts::ZonedDateTime)
     if isVariable(dfg, lbl)
-        return updateVariable!(
-            dfg,
-            setTimestamp(getVariable(dfg, lbl), ts; verbose = false),
-        )
+        return mergeVariable!(dfg, setTimestamp(getVariable(dfg, lbl), ts; verbose = false))
     else
-        return updateFactor!(dfg, setTimestamp(getFactor(dfg, lbl), ts))
+        return mergeFactor!(dfg, setTimestamp(getFactor(dfg, lbl), ts))
     end
 end
 

src/services/DFGVariable.jl

@@ -365,7 +365,7 @@ end
 Set the timestamp of a VariableCompute object returning a new VariableCompute.
 Note:
 Since the `timestamp` field is not mutable `setTimestamp` returns a new variable with the updated timestamp (note the absence of `!`).
-Use [`updateVariable!`](@ref) on the returened variable to update it in the factor graph if needed. Alternatively use [`setTimestamp!`](@ref).
+Use [`mergeVariable!`](@ref) on the returened variable to update it in the factor graph if needed. Alternatively use [`setTimestamp!`](@ref).
 See issue #315.
 """
 function setTimestamp(v::VariableCompute, ts::ZonedDateTime; verbose::Bool = true)
@@ -541,7 +541,7 @@ function addMetadata!(dfg::AbstractDFG, label::Symbol, pair::Pair{Symbol, <:Smal
     v = getVariable(dfg, label)
     haskey(v.smallData, pair.first) && error("$(pair.first) already exists.")
     push!(v.smallData, pair)
-    updateVariable!(dfg, v)
+    mergeVariable!(dfg, v)
     return v.smallData #or pair TODO
 end
 
@@ -560,7 +560,7 @@ function updateMetadata!(
         !haskey(v.smallData, pair.first) &&
         @warn("$(pair.first) does not exist, adding.")
     push!(v.smallData, pair)
-    updateVariable!(dfg, v)
+    mergeVariable!(dfg, v)
     return v.smallData #or pair TODO
 end
 
@@ -571,7 +571,7 @@ Delete a Metadata entry at `key` for variable `label` in `dfg`
 function deleteMetadata!(dfg::AbstractDFG, label::Symbol, key::Symbol)
     v = getVariable(dfg, label)
     pop!(v.smallData, key)
-    updateVariable!(dfg, v)
+    mergeVariable!(dfg, v)
     return 1
 end
 
@@ -591,7 +591,7 @@ Empty all Metadata from variable `label` in `dfg`
 function emptyMetadata!(dfg::AbstractDFG, label::Symbol)
     v = getVariable(dfg, label)
     empty!(v.smallData)
-    updateVariable!(dfg, v)
+    mergeVariable!(dfg, v)
     return v.smallData #or pair TODO
 end
 

test/GraphsDFGSummaryTypes.jl

@@ -121,14 +121,14 @@ end
         testTimestamp = now(localzone())
         v1ts = setTimestamp(v1, testTimestamp)
         @test getTimestamp(v1ts) == testTimestamp
-        #follow with updateVariable!(fg, v1ts)
-        # setTimestamp!(v1, testTimestamp) not implemented, we can do an setTimestamp() updateVariable!() for a setTimestamp!(dfg, v1, testTimestamp)
-        @test_throws MethodError setTimestamp!(v1, testTimestamp)
+        #follow with mergeVariable!(fg, v1ts)
+        # setTimestamp!(v1, testTimestamp) not implemented, we can do an setTimestamp() mergeVariable!() for a setTimestamp!(dfg, v1, testTimestamp)
+        @test_throws MethodError DFG.setTimestamp!(v1, testTimestamp)
 
         f1ts = setTimestamp(f1, testTimestamp)
         @test !(f1ts === f1)
         @test getTimestamp(f1ts) == testTimestamp
-        @test_throws MethodError setTimestamp!(v1, testTimestamp)
+        @test_throws MethodError DFG.setTimestamp!(v1, testTimestamp)
     end
 end