Deprecate and change verb update to merge (#1135)

* update -> merge for Variable and Factor

* update -> merge for Blobentry and State

---------

Co-authored-by: Johannes Terblanche <Affie@users.noreply.github.com>

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

docs/src/GraphData.md

@@ -47,7 +47,6 @@ Labels are the principle identifier of a variable or factor.
 Each variable or factor can have a timestamp associated with it.
 
 - [`getTimestamp`](@ref)
-- [`setTimestamp!`](@ref)
 
 
 #### Tags
@@ -129,7 +128,7 @@ Related functions:
 - [`listVariableSolverData`](@ref)
 - [`getVariableSolverData`](@ref)
 - [`addVariableSolverData!`](@ref)
-- [`updateVariableSolverData!`](@ref)
+- [`mergeVariableState!`](@ref)
 - [`deleteVariableSolverData!`](@ref)
 - [`mergeVariableSolverData!`](@ref)
 

src/DataBlobs/services/BlobEntry.jl

@@ -181,20 +181,21 @@ end
 
 """
     $(SIGNATURES)
-Update data entry
-
-DevNote
-- DF, unclear if `update` verb is applicable in this case, see #404
+Update a Blobentry in the factor graph.
+If the Blobentry does not exist, it will be added.
+Notes:
 """
-function updateBlobEntry!(var::AbstractDFGVariable, bde::BlobEntry)
-    !haskey(var.dataDict, bde.label) &&
-        (@warn "$(bde.label) does not exist in variable $(getLabel(var)), adding")
-    var.dataDict[bde.label] = bde
-    return bde
+function mergeBlobentry!(var::AbstractDFGVariable, bde::BlobEntry)
+    if !haskey(var.dataDict, bde.label)
+        addBlobEntry!(var, bde)
+    else
+        var.dataDict[bde.label] = bde
+    end
+    return 1
 end
-function updateBlobEntry!(dfg::AbstractDFG, label::Symbol, bde::BlobEntry)
+function mergeBlobentry!(dfg::AbstractDFG, label::Symbol, bde::BlobEntry)
     # !isVariable(dfg, label) && return nothing
-    return updateBlobEntry!(getVariable(dfg, label), bde)
+    return mergeBlobentry!(getVariable(dfg, label), bde)
 end
 
 """

src/DataBlobs/services/BlobStores.jl

@@ -25,7 +25,7 @@ function addBlob! end
 """
 Update a blob to the blob store or dfg with the given entry.
 Related
-[`updateBlobEntry!`](@ref)
+[`mergeBlobentry!`](@ref)
 
 $(METHODLIST)
 

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

@@ -25,7 +25,7 @@ function addData! end
 """
 Update a blob entry or blob to the blob store or dfg.
 Related
-[`updateBlobEntry!`](@ref)
+[`mergeBlobentry!`](@ref)
 
 $(METHODLIST)
 """
@@ -247,9 +247,9 @@ function updateData!(
 )
     checkhash && assertHash(entry, blob; hashfunction)
     # order of ops with unknown new blobId not tested
-    de = updateBlobEntry!(dfg, label, entry)
+    mergeBlobentry!(dfg, label, entry)
     db = updateBlob!(dfg, de, blob)
-    return de => db
+    return 2
 end
 
 function updateData!(
@@ -269,10 +269,9 @@ function updateData!(
         origin = buildSourceString(dfg, label),
         _version = string(_getDFGVersion()),
     )
-
-    de = updateBlobEntry!(dfg, label, newEntry)
-    db = updateBlob!(blobstore, de, blob)
-    return de => db
+    mergeBlobentry!(dfg, label, newEntry)
+    updateBlob!(blobstore, newEntry, blob)
+    return 2
 end
 
 function deleteData!(dfg::AbstractDFG, vLbl::Symbol, bLbl::Symbol)

src/Deprecated.jl

@@ -3,8 +3,151 @@
 ##=================================================================================
 
 @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{T}, data::T, ::String) where {T} addBlob!(
+    store,
+    uuid4(),
+    data,
+)
+
+@deprecate updateVariable!(args...) mergeVariable!(args...)
+@deprecate updateFactor!(args...) mergeFactor!(args...)
+
+@deprecate updateBlobEntry!(args...) mergeBlobentry!(args...)
+@deprecate updateGraphBlobEntry!(args...) mergeGraphBlobentry!(args...)
+@deprecate updateAgentBlobEntry!(args...) mergeAgentBlobentry!(args...)
+
+export updateVariableSolverData!
+
+#TODO possibly completely deprecated or not exported until update verb is standardized
+function updateVariableSolverData!(
+    dfg::AbstractDFG,
+    variablekey::Symbol,
+    vnd::VariableNodeData,
+    useCopy::Bool = false,
+    fields::Vector{Symbol} = Symbol[];
+    warn_if_absent::Bool = true,
+)
+    #This is basically just setSolverData
+    var = getVariable(dfg, variablekey)
+    warn_if_absent &&
+        !haskey(var.solverDataDict, vnd.solveKey) &&
+        @warn "VariableNodeData '$(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) &&
+        length(fields) != 0
+    # @error useExisting vnd.solveKey
+    if useExisting
+        # change multiple pointers inside the VND var.solverDataDict[solvekey]
+        for field in fields
+            destField = getfield(var.solverDataDict[vnd.solveKey], field)
+            srcField = getfield(usevnd, field)
+            if isa(destField, Array) && size(destField) == size(srcField)
+                # use broadcast (in-place operation)
+                destField .= srcField
+            else
+                # change pointer of destination VND object member
+                setfield!(var.solverDataDict[vnd.solveKey], field, srcField)
+            end
+        end
+    else
+        # change a single pointer in var.solverDataDict
+        var.solverDataDict[vnd.solveKey] = usevnd
+    end
+
+    return var.solverDataDict[vnd.solveKey]
+end
+
+function updateVariableSolverData!(
+    dfg::AbstractDFG,
+    variablekey::Symbol,
+    vnd::VariableNodeData,
+    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
+        )
+        usevnd = useCopy ? deepcopy(vnd) : vnd
+        usevnd.solveKey = solveKey
+        return updateVariableSolverData!(
+            dfg,
+            variablekey,
+            usevnd,
+            useCopy,
+            fields;
+            warn_if_absent = warn_if_absent,
+        )
+    else
+        return updateVariableSolverData!(
+            dfg,
+            variablekey,
+            vnd,
+            useCopy,
+            fields;
+            warn_if_absent = warn_if_absent,
+        )
+    end
+end
+
+function updateVariableSolverData!(
+    dfg::AbstractDFG,
+    sourceVariable::VariableCompute,
+    solveKey::Symbol = :default,
+    useCopy::Bool = false,
+    fields::Vector{Symbol} = Symbol[];
+    warn_if_absent::Bool = true,
+)
+    #
+    vnd = getSolverData(sourceVariable, solveKey)
+    # 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"
+    return updateVariableSolverData!(
+        dfg,
+        sourceVariable.label,
+        vnd,
+        useCopy,
+        fields;
+        warn_if_absent = warn_if_absent,
+    )
+end
+
+function updateVariableSolverData!(
+    dfg::AbstractDFG,
+    sourceVariables::Vector{<:VariableCompute},
+    solveKey::Symbol = :default,
+    useCopy::Bool = false,
+    fields::Vector{Symbol} = Symbol[];
+    warn_if_absent::Bool = true,
+)
+    #I think cloud would do this in bulk for speed
+    for var in sourceVariables
+        updateVariableSolverData!(
+            dfg,
+            var.label,
+            getSolverData(var, solveKey),
+            useCopy,
+            fields;
+            warn_if_absent = warn_if_absent,
+        )
+    end
+end
 
 ## ================================================================================
 ## Deprecated in v0.25
@@ -69,4 +212,3 @@ DFGSummary(args) = error("DFGSummary is deprecated")
 )
 
 @deprecate lsfWho(dfg::AbstractDFG, type::Symbol) lsf(dfg, getfield(Main, type))
-

src/DistributedFactorGraphs.jl

@@ -79,13 +79,13 @@ export getGraphBlobEntry,
     getGraphBlobEntries,
     addGraphBlobEntry!,
     addGraphBlobEntries!,
-    updateGraphBlobEntry!,
+    mergeGraphBlobentry!,
     deleteGraphBlobEntry!,
     getAgentBlobEntry,
     getAgentBlobEntries,
     addAgentBlobEntry!,
     addAgentBlobEntries!,
-    updateAgentBlobEntry!,
+    mergeAgentBlobentry!,
     deleteAgentBlobEntry!,
     listGraphBlobEntries,
     listAgentBlobEntries
@@ -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
 
@@ -186,7 +186,7 @@ export getMetadata,
 # CRUD & SET
 export getVariableSolverData,
     addVariableSolverData!,
-    updateVariableSolverData!,
+    mergeVariableState!,
     deleteVariableSolverData!,
     listVariableSolverData,
     mergeVariableSolverData!,
@@ -228,7 +228,7 @@ export hasBlobEntry,
     getBlobEntryFirst,
     addBlobEntry!,
     addBlobEntries!,
-    updateBlobEntry!,
+    mergeBlobentry!,
     deleteBlobEntry!,
     listBlobEntrySequence,
     mergeBlobEntries!

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}}