From eaea68c09d712532a2ad73338b0298da43e0e37a Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Mon, 31 Aug 2020 11:41:33 +0200 Subject: [PATCH] Add warn_if_absent to update verbs (#648) --- src/CloudGraphsDFG/services/CloudGraphsDFG.jl | 48 +++++++++++-------- src/Deprecated.jl | 21 +++++++- src/LightDFG/services/LightDFG.jl | 14 +++--- src/services/DFGVariable.jl | 46 +++++++++--------- 4 files changed, 78 insertions(+), 51 deletions(-) diff --git a/src/CloudGraphsDFG/services/CloudGraphsDFG.jl b/src/CloudGraphsDFG/services/CloudGraphsDFG.jl index f443542b..a887d639 100644 --- a/src/CloudGraphsDFG/services/CloudGraphsDFG.jl +++ b/src/CloudGraphsDFG/services/CloudGraphsDFG.jl @@ -164,15 +164,15 @@ function addVariable!(dfg::CloudGraphsDFG, variable::DFGVariable) error("Variable '$(variable.label)' already exists in the factor graph") end - ret = updateVariable!(dfg, variable, skipAddError=true) + ret = updateVariable!(dfg, variable, warn_if_absent=false) # Do a variable update return ret end -function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; skipAddError::Bool=false) +function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; warn_if_absent::Bool=true) exist = exists(dfg, variable) - !skipAddError && !exist && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding" + warn_if_absent && !exist && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding" # Create/update the base variable # NOTE: We are not merging the variable.tags into the labels anymore. We can index by that but not @@ -196,7 +196,7 @@ function updateVariable!(dfg::CloudGraphsDFG, variable::DFGVariable; skipAddErro # length(result.results[1]["data"][1]["row"]) != 1 && error("Cannot update or add variable '$(getLabel(variable))'") # Merge the PPE's, SolverData, and BigData - mergeVariableData!(dfg, variable; currentTransaction=tx, skipExistenceCheck=true) + mergeVariableData!(dfg, variable; currentTransaction=tx, warn_if_absent=false) commit(tx) catch ex @@ -220,7 +220,7 @@ function addFactor!(dfg::CloudGraphsDFG, factor::DFGFactor) end # Do a variable update - return updateFactor!(dfg, factor, skipAddError=true) + return updateFactor!(dfg, factor, warn_if_absent=false) end function getVariable(dfg::CloudGraphsDFG, label::Union{Symbol, String}) @@ -247,8 +247,8 @@ function getVariable(dfg::CloudGraphsDFG, label::Union{Symbol, String}) return variable end -function mergeVariableData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, skipExistenceCheck::Bool=false) - if !skipExistenceCheck & !exists(dfg, sourceVariable, currentTransaction=currentTransaction) +function mergeVariableData!(dfg::CloudGraphsDFG, sourceVariable::DFGVariable; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, warn_if_absent::Bool=true) + if warn_if_absent && !exists(dfg, sourceVariable, currentTransaction=currentTransaction) error("Source variable '$(sourceVariable.label)' doesn't exist in the graph.") end for (k,v) in sourceVariable.ppeDict @@ -273,9 +273,9 @@ function getFactor(dfg::CloudGraphsDFG, label::Union{Symbol, String}) return rebuildFactorMetadata!(dfg, unpackFactor(dfg, props)) end -function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor; skipAddError::Bool=false) +function updateFactor!(dfg::CloudGraphsDFG, factor::DFGFactor; warn_if_absent::Bool=true) exist = exists(dfg, factor) - !skipAddError && !exist && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding" + warn_if_absent && !exist && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding" if exist # Check that the neighbors are the same @@ -599,10 +599,11 @@ end function updatePPE!( dfg::CloudGraphsDFG, variablekey::Symbol, - ppe::P; - currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::P where - {P <: AbstractPointParametricEst} - if !(ppe.solveKey in listPPEs(dfg, variablekey, currentTransaction=currentTransaction)) + ppe::AbstractPointParametricEst; + currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, + warn_if_absent::Bool=true) + + if warn_if_absent && !(ppe.solveKey in listPPEs(dfg, variablekey, currentTransaction=currentTransaction)) @warn "PPE '$(ppe.solveKey)' does not exist, adding" end softType = getSofttype(dfg, variablekey, currentTransaction=currentTransaction) @@ -618,10 +619,13 @@ function updatePPE!( currentTransaction=currentTransaction)) end -function updatePPE!(dfg::CloudGraphsDFG, sourceVariables::Vector{<:DFGVariable}, ppekey::Symbol=:default; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing) +function updatePPE!(dfg::CloudGraphsDFG, sourceVariables::Vector{<:DFGVariable}, ppekey::Symbol=:default; + currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, + warn_if_absent::Bool=true) + tx = currentTransaction == nothing ? transaction(dfg.neo4jInstance.connection) : currentTransaction for var in sourceVariables - updatePPE!(dfg, var.label, getPPE(var, ppekey), currentTransaction=tx) + updatePPE!(dfg, var.label, getPPE(var, ppekey), currentTransaction=tx, warn_if_absent=warn_if_absent) end if currentTransaction == nothing result = commit(tx) @@ -694,8 +698,10 @@ function addDataEntry!(dfg::CloudGraphsDFG, label::Symbol, bde::BlobStoreEntry; packed) end -function updateDataEntry!(dfg::CloudGraphsDFG, label::Symbol, bde::BlobStoreEntry; currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing) - if !(bde.label in listDataEntries(dfg, label, currentTransaction=currentTransaction)) +function updateDataEntry!(dfg::CloudGraphsDFG, label::Symbol, bde::BlobStoreEntry; + currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing, + warn_if_absent::Bool=true) + if warn_if_absent && !(bde.label in listDataEntries(dfg, label, currentTransaction=currentTransaction)) @warn "Data label '$(bde.label)' does not exist, adding" end packed = _matchmergeVariableSubnode!( @@ -773,8 +779,9 @@ function updateVariableSolverData!(dfg::CloudGraphsDFG, vnd::VariableNodeData, useCopy::Bool=true, fields::Vector{Symbol}=Symbol[]; + warn_if_absent::Bool=true, currentTransaction::Union{Nothing, Neo4j.Transaction}=nothing)::VariableNodeData - if !(vnd.solveKey in listVariableSolverData(dfg, variablekey, currentTransaction=currentTransaction)) + if warn_if_absent && !(vnd.solveKey in listVariableSolverData(dfg, variablekey, currentTransaction=currentTransaction)) @warn "Solver data '$(vnd.solveKey)' does not exist, adding rather than updating." end # TODO: Update this to use the selective parameters from fields. @@ -847,10 +854,11 @@ function updateVariableSolverData!(dfg::CloudGraphsDFG, sourceVariables::Vector{<:DFGVariable}, solvekey::Symbol=:default, useCopy::Bool=true, - fields::Vector{Symbol}=Symbol[] ) + fields::Vector{Symbol}=Symbol[]; + warn_if_absent::Bool=true ) #TODO: Do in bulk for speed. for var in sourceVariables - updateVariableSolverData!(dfg, var.label, getSolverData(var, solvekey), solvekey, useCopy, fields) + updateVariableSolverData!(dfg, var.label, getSolverData(var, solvekey), solvekey, useCopy, fields; warn_if_absent=warn_if_absent) end end diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 96082de8..a0caec6d 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -104,5 +104,24 @@ end @deprecate setSmallData!(v::DFGVariable, smallData::Dict{String, String}) setSmallData!(v, createSymbolDict(smallData)) +#update deprecation with warn_if_absent +function updateVariableSolverData!(dfg::AbstractDFG, + variablekey::Symbol, + vnd::VariableNodeData, + useCopy::Bool, + fields::Vector{Symbol}, + verbose::Bool) + Base.depwarn("updateVariableSolverData! argument verbose is deprecated in favor of keyword argument `warn_if_absent`, see #643", :updateVariableSolverData!) + updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields; warn_if_absent = verbose) +end - +function updateVariableSolverData!(dfg::AbstractDFG, + variablekey::Symbol, + vnd::VariableNodeData, + solveKey::Symbol, + useCopy::Bool, + fields::Vector{Symbol}, + verbose::Bool) + Base.depwarn("updateVariableSolverData! argument verbose is deprecated in favor of keyword argument `warn_if_absent`, see #643", :updateVariableSolverData!) + updateVariableSolverData!(dfg, variablekey, vnd, solveKey, useCopy, fields; warn_if_absent = verbose) +end diff --git a/src/LightDFG/services/LightDFG.jl b/src/LightDFG/services/LightDFG.jl index 2bda4d8c..918228af 100644 --- a/src/LightDFG/services/LightDFG.jl +++ b/src/LightDFG/services/LightDFG.jl @@ -90,11 +90,11 @@ function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F}, f end function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F}, - factor::AbstractDFGFactor)::F where F <: AbstractDFGFactor + factor::AbstractDFGFactor) where F <: AbstractDFGFactor return addFactor!(dfg, F(factor)) end -function getVariable(dfg::LightDFG, label::Symbol)::AbstractDFGVariable +function getVariable(dfg::LightDFG, label::Symbol) if !haskey(dfg.g.variables, label) error("Variable label '$(label)' does not exist in the factor graph") end @@ -102,25 +102,25 @@ function getVariable(dfg::LightDFG, label::Symbol)::AbstractDFGVariable return dfg.g.variables[label] end -function getFactor(dfg::LightDFG, label::Symbol)::AbstractDFGFactor +function getFactor(dfg::LightDFG, label::Symbol) if !haskey(dfg.g.factors, label) error("Factor label '$(label)' does not exist in the factor graph") end return dfg.g.factors[label] end -function updateVariable!(dfg::LightDFG, variable::V)::V where V <: AbstractDFGVariable +function updateVariable!(dfg::LightDFG, variable::AbstractDFGVariable; warn_if_absent::Bool=true) if !haskey(dfg.g.variables, variable.label) - @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding" + warn_if_absent && @warn "Variable label '$(variable.label)' does not exist in the factor graph, adding" return addVariable!(dfg, variable) end dfg.g.variables[variable.label] = variable return variable end -function updateFactor!(dfg::LightDFG, factor::F)::F where F <: AbstractDFGFactor +function updateFactor!(dfg::LightDFG, factor::AbstractDFGFactor; warn_if_absent::Bool=true) if !haskey(dfg.g.factors, factor.label) - @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding" + warn_if_absent && @warn "Factor label '$(factor.label)' does not exist in the factor graph, adding" return addFactor!(dfg, factor) end diff --git a/src/services/DFGVariable.jl b/src/services/DFGVariable.jl index 02b8217b..a270aa57 100644 --- a/src/services/DFGVariable.jl +++ b/src/services/DFGVariable.jl @@ -345,9 +345,9 @@ end $(SIGNATURES) Update a small data pair `key=>value` for variable `label` in `dfg` """ -function updateSmallData!(dfg::AbstractDFG, label::Symbol, pair::Pair{Symbol, <:SmallDataTypes}) +function updateSmallData!(dfg::AbstractDFG, label::Symbol, pair::Pair{Symbol, <:SmallDataTypes}; warn_if_absent::Bool=true) v = getVariable(dfg, label) - !haskey(v.smallData, pair.first) && @warn("$(pair.first) does not exist, adding.") + warn_if_absent && !haskey(v.smallData, pair.first) && @warn("$(pair.first) does not exist, adding.") push!(v.smallData, pair) updateVariable!(dfg, v) return v.smallData #or pair TODO @@ -478,13 +478,12 @@ function updateVariableSolverData!(dfg::AbstractDFG, variablekey::Symbol, vnd::VariableNodeData, useCopy::Bool=true, - fields::Vector{Symbol}=Symbol[], - verbose::Bool=true ) + fields::Vector{Symbol}=Symbol[]; + warn_if_absent::Bool=true) #This is basically just setSolverData var = getVariable(dfg, variablekey) - if verbose && !haskey(var.solverDataDict, vnd.solveKey) - @warn "VariableNodeData '$(vnd.solveKey)' does not exist, adding" - end + + 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 = useCopy ? deepcopy(vnd) : vnd @@ -510,23 +509,22 @@ function updateVariableSolverData!(dfg::AbstractDFG, return var.solverDataDict[vnd.solveKey] end - function updateVariableSolverData!(dfg::AbstractDFG, variablekey::Symbol, vnd::VariableNodeData, solveKey::Symbol, useCopy::Bool=true, - fields::Vector{Symbol}=Symbol[], - verbose::Bool=true) + 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, verbose) + return updateVariableSolverData!(dfg, variablekey, usevnd, useCopy, fields; warn_if_absent=warn_if_absent) else - return updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields, verbose) + return updateVariableSolverData!(dfg, variablekey, vnd, useCopy, fields; warn_if_absent=warn_if_absent) end end @@ -535,17 +533,19 @@ updateVariableSolverData!(dfg::AbstractDFG, sourceVariable::DFGVariable, solveKey::Symbol=:default, useCopy::Bool=true, - fields::Vector{Symbol}=Symbol[] ) = - updateVariableSolverData!(dfg, sourceVariable.label, getSolverData(sourceVariable, solveKey), useCopy, fields) + fields::Vector{Symbol}=Symbol[]; + warn_if_absent::Bool=true ) = + updateVariableSolverData!(dfg, sourceVariable.label, getSolverData(sourceVariable, solveKey), useCopy, fields; warn_if_absent=warn_if_absent) function updateVariableSolverData!(dfg::AbstractDFG, sourceVariables::Vector{<:DFGVariable}, solveKey::Symbol=:default, useCopy::Bool=true, - fields::Vector{Symbol}=Symbol[] ) + 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) + updateVariableSolverData!(dfg, var.label, getSolverData(var, solveKey), useCopy, fields; warn_if_absent=warn_if_absent) end end @@ -563,7 +563,7 @@ function deepcopySolvekeys!(dfg::AbstractDFG, for x in labels sd = deepcopy(getSolverData(getVariable(dfg,x), src)) sd.solveKey = dest - updateVariableSolverData!(dfg, x, sd, true, Symbol[], verbose ) + updateVariableSolverData!(dfg, x, sd, true, Symbol[]; warn_if_absent=verbose ) end end const deepcopySupersolve! = deepcopySolvekeys! @@ -670,9 +670,9 @@ addPPE!(dfg::AbstractDFG, sourceVariable::DFGVariable, ppekey::Symbol=:default) $(SIGNATURES) Update PPE data if it exists, otherwise add it -- one call per `key::Symbol=:default`. """ -function updatePPE!(dfg::AbstractDFG, variablekey::Symbol, ppe::P)::P where {P <: AbstractPointParametricEst} +function updatePPE!(dfg::AbstractDFG, variablekey::Symbol, ppe::AbstractPointParametricEst; warn_if_absent::Bool=true) var = getVariable(dfg, variablekey) - if !haskey(var.ppeDict, ppe.solveKey) + if warn_if_absent && !haskey(var.ppeDict, ppe.solveKey) @warn "PPE '$(ppe.solveKey)' does not exist, adding" end #for InMemoryDFGTypes, cloud would update here @@ -685,17 +685,17 @@ end Update PPE data if it exists, otherwise add it. NOTE: Copies the PPE data. """ -updatePPE!(dfg::AbstractDFG, sourceVariable::VariableDataLevel1, ppekey::Symbol=:default) = - updatePPE!(dfg, sourceVariable.label, deepcopy(getPPE(sourceVariable, ppekey))) +updatePPE!(dfg::AbstractDFG, sourceVariable::VariableDataLevel1, ppekey::Symbol=:default; warn_if_absent::Bool=true) = + updatePPE!(dfg, sourceVariable.label, deepcopy(getPPE(sourceVariable, ppekey)); warn_if_absent=warn_if_absent) """ $(SIGNATURES) Update PPE data if it exists, otherwise add it. """ -function updatePPE!(dfg::AbstractDFG, sourceVariables::Vector{<:VariableDataLevel1}, ppekey::Symbol=:default) +function updatePPE!(dfg::AbstractDFG, sourceVariables::Vector{<:VariableDataLevel1}, ppekey::Symbol=:default; warn_if_absent::Bool=true) #I think cloud would do this in bulk for speed for var in sourceVariables - updatePPE!(dfg, var.label, getPPE(dfg, var, ppekey)) + updatePPE!(dfg, var.label, getPPE(dfg, var, ppekey); warn_if_absent=warn_if_absent) end end