diff --git a/NEWS.md b/NEWS.md index df4939f2..d983659a 100644 --- a/NEWS.md +++ b/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`. diff --git a/docs/src/BuildingGraphs.md b/docs/src/BuildingGraphs.md index 9498b6ec..561aa731 100644 --- a/docs/src/BuildingGraphs.md +++ b/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 diff --git a/docs/src/GraphData.md b/docs/src/GraphData.md index c595584c..57f1ddf6 100644 --- a/docs/src/GraphData.md +++ b/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) diff --git a/src/DataBlobs/services/BlobEntry.jl b/src/DataBlobs/services/BlobEntry.jl index e7cbb99c..bd261263 100644 --- a/src/DataBlobs/services/BlobEntry.jl +++ b/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 """ diff --git a/src/DataBlobs/services/BlobStores.jl b/src/DataBlobs/services/BlobStores.jl index a94c0ab0..9d73af82 100644 --- a/src/DataBlobs/services/BlobStores.jl +++ b/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) diff --git a/src/DataBlobs/services/HelpersDataWrapEntryBlob.jl b/src/DataBlobs/services/HelpersDataWrapEntryBlob.jl index 509314f7..a61ce760 100644 --- a/src/DataBlobs/services/HelpersDataWrapEntryBlob.jl +++ b/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) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 752defa3..bf43c331 100644 --- a/src/Deprecated.jl +++ b/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)) - diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 2d811f0e..eda5775a 100644 --- a/src/DistributedFactorGraphs.jl +++ b/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! diff --git a/src/GraphsDFG/GraphsDFG.jl b/src/GraphsDFG/GraphsDFG.jl index dbb43fa6..f6ddcb4e 100644 --- a/src/GraphsDFG/GraphsDFG.jl +++ b/src/GraphsDFG/GraphsDFG.jl @@ -27,8 +27,8 @@ import ...DistributedFactorGraphs: exists, isVariable, isFactor, - updateVariable!, - updateFactor!, + mergeVariable!, + mergeFactor!, deleteVariable!, deleteFactor!, getVariables, diff --git a/src/GraphsDFG/services/GraphsDFG.jl b/src/GraphsDFG/services/GraphsDFG.jl index 51a9e0ee..f74e0919 100644 --- a/src/GraphsDFG/services/GraphsDFG.jl +++ b/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}} diff --git a/src/entities/DFGVariable.jl b/src/entities/DFGVariable.jl index edc00c4c..c6f55c00 100644 --- a/src/entities/DFGVariable.jl +++ b/src/entities/DFGVariable.jl @@ -303,15 +303,15 @@ Base.@kwdef struct VariableCompute{T <: InferenceVariable, P, N} <: AbstractDFGV Accessors: [`addPPE!`](@ref), [`updatePPE!`](@ref), and [`deletePPE!`](@ref)""" ppeDict::Dict{Symbol, AbstractPointParametricEst} = Dict{Symbol, AbstractPointParametricEst}() - """Dictionary of solver data. May be a subset of all solutions if a solver key was specified in the get call. - Accessors: [`addVariableSolverData!`](@ref), [`updateVariableSolverData!`](@ref), and [`deleteVariableSolverData!`](@ref)""" + """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}}() """Dictionary of small data associated with this variable. Accessors: [`getMetadata`](@ref), [`setMetadata!`](@ref)""" smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}() """Dictionary of large data associated with this variable. - Accessors: [`addBlobEntry!`](@ref), [`getBlobEntry`](@ref), [`updateBlobEntry!`](@ref), and [`deleteBlobEntry!`](@ref)""" + Accessors: [`addBlobEntry!`](@ref), [`getBlobEntry`](@ref), [`mergeBlobentry!`](@ref), and [`deleteBlobEntry!`](@ref)""" dataDict::Dict{Symbol, BlobEntry} = Dict{Symbol, BlobEntry}() """Solvable flag for the variable. Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)""" @@ -405,7 +405,7 @@ Base.@kwdef struct VariableSummary <: AbstractDFGVariable Accessor: [`getVariableType`](@ref)""" variableTypeName::Symbol """Dictionary of large data associated with this variable. - Accessors: [`addBlobEntry!`](@ref), [`getBlobEntry`](@ref), [`updateBlobEntry!`](@ref), and [`deleteBlobEntry!`](@ref)""" + Accessors: [`addBlobEntry!`](@ref), [`getBlobEntry`](@ref), [`mergeBlobentry!`](@ref), and [`deleteBlobEntry!`](@ref)""" dataDict::Dict{Symbol, BlobEntry} end diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl index 914cc778..1c77fbce 100644 --- a/src/services/AbstractDFG.jl +++ b/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 @@ -236,14 +236,14 @@ function getGraphBlobEntry end function getGraphBlobEntries end function addGraphBlobEntry! end function addGraphBlobEntries! end -function updateGraphBlobEntry! end +function mergeGraphBlobentry! end function deleteGraphBlobEntry! end function getAgentBlobEntry end function getAgentBlobEntries end function addAgentBlobEntry! end function addAgentBlobEntries! end -function updateAgentBlobEntry! end +function mergeAgentBlobentry! end function deleteAgentBlobEntry! end function getModelBlobEntry end @@ -392,21 +392,20 @@ end """ $(SIGNATURES) -Update a complete VariableCompute in the DFG. +Merge a variable into the DFG. If a variable with the same label exists, it will be overwritten; +otherwise, the variable will be added to the graph. """ -function updateVariable!( - dfg::G, - variable::V, -) where {G <: AbstractDFG, V <: AbstractDFGVariable} - return error("updateVariable! not implemented for $(typeof(dfg))") +function mergeVariable!(dfg::AbstractDFG, variable::AbstractDFGVariable) + return error("mergeVariable! not implemented for $(typeof(dfg))") end """ $(SIGNATURES) -Update a complete FactorCompute in the DFG. +Merge a factor into the DFG. If a factor with the same label exists, it will be overwritten; +otherwise, the factor will be added to the graph. """ -function updateFactor!(dfg::G, factor::F) where {G <: AbstractDFG, F <: AbstractDFGFactor} - return error("updateFactor! not implemented for $(typeof(dfg))") +function mergeFactor!(dfg::AbstractDFG, factor::AbstractDFGFactor) + return error("mergeFactor! not implemented for $(typeof(dfg))") end """ @@ -420,12 +419,8 @@ end $(SIGNATURES) Delete a FactorCompute from the DFG using its label. """ -function deleteFactor!( - dfg::G, - label::Symbol; - suppressGetFactor::Bool = false, -) where {G <: AbstractDFG} - return error("deleteFactors not implemented for $(typeof(dfg))") +function deleteFactor!(dfg::AbstractDFG, label::Symbol) + return error("deleteFactor not implemented for $(typeof(dfg))") end """ @@ -1097,7 +1092,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 +1114,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 +1453,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 diff --git a/src/services/CommonAccessors.jl b/src/services/CommonAccessors.jl index e5df465d..856df9f2 100644 --- a/src/services/CommonAccessors.jl +++ b/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 diff --git a/src/services/DFGVariable.jl b/src/services/DFGVariable.jl index 946450cb..fdfae0ea 100644 --- a/src/services/DFGVariable.jl +++ b/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 @@ -694,139 +694,22 @@ end """ $(SIGNATURES) -Update variable solver data if it exists, otherwise add it. - -Notes: -- `useCopy=true` to copy solver data and keep separate memory. -- Use `fields` to updated only a few VND.fields while adhering to `useCopy`. +Update the variable state if it exists, otherwise add it. Related -mergeVariableSolverData! +mergeVariableStates! """ -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 mergeVariableState!(dfg::AbstractDFG, variablekey::Symbol, vnd::VariableNodeData) + v = getVariable(dfg, variablekey) -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, - ) + if !haskey(v.solverDataDict, vnd.solveKey) + addVariableSolverData!(dfg, variablekey, vnd) else - return updateVariableSolverData!( - dfg, - variablekey, - vnd, - useCopy, - fields; - warn_if_absent = warn_if_absent, - ) + v.solverDataDict[vnd.solveKey] = usevnd 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 + return 1 end """ diff --git a/test/GraphsDFGSummaryTypes.jl b/test/GraphsDFGSummaryTypes.jl index 95a8a357..73c38f1d 100644 --- a/test/GraphsDFGSummaryTypes.jl +++ b/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 diff --git a/test/fileDFGTests.jl b/test/fileDFGTests.jl index 37f82e3c..1b8593af 100644 --- a/test/fileDFGTests.jl +++ b/test/fileDFGTests.jl @@ -69,7 +69,7 @@ using UUIDs map(v -> addPPE!(dfg, getLabel(v), deepcopy(ppe2)), verts) #call update to set it on cloud - updateVariable!.(dfg, verts) + mergeVariable!.(dfg, verts) facts = map( n -> addFactor!( @@ -82,7 +82,7 @@ using UUIDs map(f -> setSolvable!(f, Int(round(rand()))), facts) map(f -> f.solverData.eliminated = rand() > 0.5, facts) map(f -> f.solverData.potentialused = rand() > 0.5, facts) - updateFactor!.(dfg, facts) + mergeFactor!.(dfg, facts) #test multihypo addFactor!( diff --git a/test/iifInterfaceTests.jl b/test/iifInterfaceTests.jl index 699f0ac8..f1dae890 100644 --- a/test/iifInterfaceTests.jl +++ b/test/iifInterfaceTests.jl @@ -71,12 +71,12 @@ end # Add it to the new graph. @test addVariable!(dfg2, v1) == v1 @test addVariable!(dfg2, v2) == v2 - @test @test_logs (:warn, r"exist") match_mode = :any updateVariable!(dfg2, v3) == v3 + @test mergeVariable!(dfg2, v3) == 1 @test_throws ErrorException addVariable!(dfg2, v3) @test addFactor!(dfg2, f1) == f1 @test_throws ErrorException addFactor!(dfg2, f1) - # @test @test_logs (:warn, r"exist") updateFactor!(dfg2, f2) == f2 - @test updateFactor!(dfg2, f2) == f2 + # @test @test_logs (:warn, r"exist") mergeFactor!(dfg2, f2) == f2 + @test mergeFactor!(dfg2, f2) == 1 @test_throws ErrorException addFactor!(dfg2, f2) dv3 = deleteVariable!(dfg2, v3) @@ -176,11 +176,9 @@ end # Sets v1Prime = deepcopy(v1) - @test updateVariable!(dfg, v1Prime) == v1 #Maybe move to crud - @test updateVariable!(dfg, v1Prime) == getVariable(dfg, v1.label) + @test mergeVariable!(dfg, v1Prime) == 1 #Maybe move to crud f1Prime = deepcopy(f1) - @test updateFactor!(dfg, f1Prime) == f1 #Maybe move to crud - @test updateFactor!(dfg, f1Prime) == getFactor(dfg, f1.label) + @test mergeFactor!(dfg, f1Prime) == 1 #Maybe move to crud # Accessors @test getLabel(v1) == v1.label @@ -266,13 +264,9 @@ end @test_throws KeyError getBlobEntry(dfg, :b, :key1) #update - @test updateBlobEntry!(dfg, :a, de2_update) == de2_update + @test mergeBlobentry!(dfg, :a, de2_update) == 1 @test deepcopy(de2_update) == getBlobEntry(dfg, :a, :key2) - @test @test_logs (:warn, r"does not exist") match_mode = :any updateBlobEntry!( - dfg, - :b, - de2_update, - ) == de2_update + @test mergeBlobentry!(dfg, :b, de2_update) == 1 #list entries = getBlobEntries(dfg, :a) @@ -401,8 +395,8 @@ setSolvable!(verts[7], 1) setSolvable!(verts[8], 0) getSolverData(verts[8]).solveInProgress = 1 #call update to set it on cloud -updateVariable!(dfg, verts[7]) -updateVariable!(dfg, verts[8]) +mergeVariable!(dfg, verts[7]) +mergeVariable!(dfg, verts[8]) facts = map( n -> addFactor!( diff --git a/test/runtests.jl b/test/runtests.jl index 3e2b9432..3ccaa549 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -127,8 +127,8 @@ struct NotImplementedDFG{T} <: AbstractDFG{T} end @test_throws ErrorException getVariable(dfg, :a) @test_throws ErrorException getFactor(dfg, :a) - @test_throws ErrorException updateVariable!(dfg, v1) - @test_throws ErrorException updateFactor!(dfg, f1) + @test_throws ErrorException mergeVariable!(dfg, v1) + @test_throws ErrorException mergeFactor!(dfg, f1) @test_throws ErrorException deleteVariable!(dfg, :a) @test_throws ErrorException deleteFactor!(dfg, :a) diff --git a/test/testBlocks.jl b/test/testBlocks.jl index e98ed303..a76d2d82 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -348,12 +348,12 @@ function DFGVariableSCA() @test setTags!(v3, testTags) == Set(testTags) @test setTags!(v3, Set(testTags)) == Set(testTags) - #NOTE a variable's timestamp is considered similar to its label. setTimestamp! (not implemented) would create a new variable and call updateVariable! - v1ts = setTimestamp(v1, testTimestamp) + #NOTE a variable's timestamp is considered similar to its label. setTimestamp! (not implemented) would create a new variable and call mergeVariable! + v1ts = DFG.setTimestamp(v1, testTimestamp) @test getTimestamp(v1ts) == testTimestamp - #follow with updateVariable!(fg, v1ts) + #follow with mergeVariable!(fg, v1ts) - @test_throws MethodError setTimestamp!(v1, testTimestamp) + @test_throws MethodError DFG.setTimestamp!(v1, testTimestamp) @test setSolvable!(v1, 1) == 1 @test getSolvable(v1) == 1 @@ -433,7 +433,7 @@ function DFGFactorSCA() f1ts = setTimestamp(f1, testTimestamp) @test !(f1ts === f1) @test getTimestamp(f1ts) == testTimestamp - #follow with updateFactor!(fg, v1ts) + #follow with mergeFactor!(fg, v1ts) #TODO Should throw method error # @test_throws MethodError setTimestamp!(f1, testTimestamp) @@ -481,18 +481,12 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) @test getLabel(fg[getLabel(f1)]) == getLabel(f1) - @test @test_logs (:warn, Regex("'$(v3.label)' does not exist")) match_mode = :any updateVariable!( - fg, - v3, - ) == v3 - @test updateVariable!(fg, v3) == v3 + @test mergeVariable!(fg, v3) == 1 + @test mergeVariable!(fg, v3) == 1 @test_throws ErrorException addVariable!(fg, v3) - @test @test_logs (:warn, Regex("'$(f2.label)' does not exist")) match_mode = :any updateFactor!( - fg, - f2, - ) === f2 - @test updateFactor!(fg, f2) === f2 + @test mergeFactor!(fg, f2) == 1 + @test mergeFactor!(fg, f2) == 1 @test_throws ErrorException addFactor!(fg, f2) #TODO Graphs.jl, but look at refactoring absract @test_throws ErrorException addFactor!(fg, f2) @@ -511,7 +505,7 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) pop!(f2_mod._variableOrderSymbols) end - @test_throws ErrorException updateFactor!(fg, f2_mod) + @test_throws ErrorException mergeFactor!(fg, f2_mod) @test issetequal(lsf(fg), [:bcf1, :abf1]) @test getAddHistory(fg) == [:a, :b, :c] @@ -519,10 +513,10 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) # Extra timestamp functions https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/315 if !(v1 isa VariableSkeleton) newtimestamp = now(localzone()) - @test !(setTimestamp!(fg, :c, newtimestamp) === v3) + @test !(DFG.setTimestamp!(fg, :c, newtimestamp) === v3) @test getVariable(fg, :c) |> getTimestamp == newtimestamp - @test !(setTimestamp!(fg, :bcf1, newtimestamp) === f2) + @test !(DFG.setTimestamp!(fg, :bcf1, newtimestamp) === f2) @test getFactor(fg, :bcf1) |> getTimestamp == newtimestamp end #deletions @@ -994,10 +988,9 @@ function DataEntriesTestBlock!(fg, v2) @test_throws KeyError getBlobEntry(fg, :b, :key1) #update - @test updateBlobEntry!(fg, :a, de2_update) == de2_update + @test mergeBlobentry!(fg, :a, de2_update) == 1 @test deepcopy(de2_update) == getBlobEntry(fg, :a, :key2) - @test @test_logs (:warn, r"does not exist") updateBlobEntry!(fg, :b, de2_update) == - de2_update + @test mergeBlobentry!(fg, :b, de2_update) == 1 #list entries = getBlobEntries(fg, :a) @@ -1075,7 +1068,7 @@ function blobsStoresTestBlock!(fg) var1 = getVariable(fg, :a) var2 = getVariable(fg, :b) @test addBlobEntry!(var1, de1) == de1 - updateVariable!(fg, var1) + mergeVariable!(fg, var1) @test addBlobEntry!(fg, :a, de2) == de2 @test_throws ErrorException addBlobEntry!(var1, de1) @test de2 in getBlobEntries(fg, var1.label) @@ -1087,10 +1080,9 @@ function blobsStoresTestBlock!(fg) @test_throws KeyError getBlobEntry(fg, :b, :label1) #update - @test updateBlobEntry!(fg, :a, de2_update) == de2_update + @test mergeBlobentry!(fg, :a, de2_update) == 1 @test deepcopy(de2_update) == getBlobEntry(fg, :a, :label2) - @test @test_logs (:warn, r"does not exist") updateBlobEntry!(fg, :b, de2_update) == - de2_update + @test mergeBlobentry!(fg, :b, de2_update) == 1 #list entries = getBlobEntries(fg, :a) @@ -1149,9 +1141,7 @@ function blobsStoresTestBlock!(fg) @test data[1].hash == newData.hash #[1] # @test data[2] == newData[2] # Updating - updateData = updateData!(fg, fs, :a, newData, rand(UInt8, 50)) # convenience wrapper around updateBlob! - @test updateData[1].hash != data[1].hash - @test updateData[2] != data[2] + @test updateData!(fg, fs, :a, newData, rand(UInt8, 50)) == 2 @show bllb = DistributedFactorGraphs.incrDataLabelSuffix(fg, :a, :testing) newData2 = addData!(fg, fs.label, :a, bllb, testData) # convenience wrapper over addBlob! nbe = listBlobEntries(fg, :a) @@ -1857,7 +1847,7 @@ function FileDFGTestBlock(testDFGAPI; kwargs...) vnd.solvedCount = 2 # vnd.val[1] = [2.0;] #update - updateVariable!(dfg, v4) + mergeVariable!(dfg, v4) f45 = getFactor(dfg, :x4x5f1) fsd = f45.solverData @@ -1870,7 +1860,7 @@ function FileDFGTestBlock(testDFGAPI; kwargs...) fsd.potentialused = true fsd.solveInProgress = true #update factor - updateFactor!(dfg, f45) + mergeFactor!(dfg, f45) # Save and load the graph to test. saveDFG(dfg, filename)