Skip to content

Deprecate and change verb update to merge #1135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
4 changes: 2 additions & 2 deletions docs/src/BuildingGraphs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions docs/src/GraphData.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -129,7 +128,7 @@ Related functions:
- [`listVariableSolverData`](@ref)
- [`getVariableSolverData`](@ref)
- [`addVariableSolverData!`](@ref)
- [`updateVariableSolverData!`](@ref)
- [`mergeVariableState!`](@ref)
- [`deleteVariableSolverData!`](@ref)
- [`mergeVariableSolverData!`](@ref)

Expand Down
23 changes: 12 additions & 11 deletions src/DataBlobs/services/BlobEntry.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

"""
Expand Down
2 changes: 1 addition & 1 deletion src/DataBlobs/services/BlobStores.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
13 changes: 6 additions & 7 deletions src/DataBlobs/services/HelpersDataWrapEntryBlob.jl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"""
Update a blob entry or blob to the blob store or dfg.
Related
[`updateBlobEntry!`](@ref)
[`mergeBlobentry!`](@ref)

$(METHODLIST)
"""
Expand Down Expand Up @@ -247,9 +247,9 @@
)
checkhash && assertHash(entry, blob; hashfunction)
# order of ops with unknown new blobId not tested
de = updateBlobEntry!(dfg, label, entry)
mergeBlobentry!(dfg, label, entry)

Check warning on line 250 in src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl#L250

Added line #L250 was not covered by tests
db = updateBlob!(dfg, de, blob)
return de => db
return 2

Check warning on line 252 in src/DataBlobs/services/HelpersDataWrapEntryBlob.jl

View check run for this annotation

Codecov / codecov/patch

src/DataBlobs/services/HelpersDataWrapEntryBlob.jl#L252

Added line #L252 was not covered by tests
end

function updateData!(
Expand All @@ -269,10 +269,9 @@
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)
Expand Down
148 changes: 145 additions & 3 deletions src/Deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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!(

Check warning on line 96 in src/Deprecated.jl

View check run for this annotation

Codecov / codecov/patch

src/Deprecated.jl#L96

Added line #L96 was not covered by tests
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
Expand Down Expand Up @@ -69,4 +212,3 @@
)

@deprecate lsfWho(dfg::AbstractDFG, type::Symbol) lsf(dfg, getfield(Main, type))

14 changes: 7 additions & 7 deletions src/DistributedFactorGraphs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ export getGraphBlobEntry,
getGraphBlobEntries,
addGraphBlobEntry!,
addGraphBlobEntries!,
updateGraphBlobEntry!,
mergeGraphBlobentry!,
deleteGraphBlobEntry!,
getAgentBlobEntry,
getAgentBlobEntries,
addAgentBlobEntry!,
addAgentBlobEntries!,
updateAgentBlobEntry!,
mergeAgentBlobentry!,
deleteAgentBlobEntry!,
listGraphBlobEntries,
listAgentBlobEntries
Expand All @@ -110,8 +110,8 @@ export exists,
addFactors!,
getVariable,
getFactor,
updateVariable!,
updateFactor!,
mergeVariable!,
mergeFactor!,
deleteVariable!,
deleteFactor!,
listVariables,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -186,7 +186,7 @@ export getMetadata,
# CRUD & SET
export getVariableSolverData,
addVariableSolverData!,
updateVariableSolverData!,
mergeVariableState!,
deleteVariableSolverData!,
listVariableSolverData,
mergeVariableSolverData!,
Expand Down Expand Up @@ -228,7 +228,7 @@ export hasBlobEntry,
getBlobEntryFirst,
addBlobEntry!,
addBlobEntries!,
updateBlobEntry!,
mergeBlobentry!,
deleteBlobEntry!,
listBlobEntrySequence,
mergeBlobEntries!
Expand Down
4 changes: 2 additions & 2 deletions src/GraphsDFG/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import ...DistributedFactorGraphs:
exists,
isVariable,
isFactor,
updateVariable!,
updateFactor!,
mergeVariable!,
mergeFactor!,
deleteVariable!,
deleteFactor!,
getVariables,
Expand Down
40 changes: 16 additions & 24 deletions src/GraphsDFG/services/GraphsDFG.jl
Original file line number Diff line number Diff line change
Expand Up @@ -134,37 +134,29 @@
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)

Check warning on line 154 in src/GraphsDFG/services/GraphsDFG.jl

View check run for this annotation

Codecov / codecov/patch

src/GraphsDFG/services/GraphsDFG.jl#L153-L154

Added lines #L153 - L154 were not covered by tests
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}}
Expand Down
Loading
Loading