Custom DFG errors (#1147)

Author: Affie

NEWS.md

@@ -22,6 +22,7 @@ Listing news on any major breaking changes in DFG.  For regular changes, see int
 - Deprecate getBlobentryFirst -> getfirstBlobentry, see #1114
 - OrderedDict is no longer exported
 - FolderStore path now includes the store label.
+- Standardized error types and behaviour.
 
 # v0.26
 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`.

docs/src/func_ref.md

@@ -44,6 +44,12 @@ Modules = [DistributedFactorGraphs]
 Pages = ["entities/DFGFactor.jl"]
 ```
 
+### Error Types
+```@autodocs
+Modules = [DistributedFactorGraphs]
+Pages = ["errors.jl"]
+```
+
 ## DFG Plots [GraphMakie.jl]
 
 ```@autodocs

src/DataBlobs/services/BlobEntry.jl

@@ -67,18 +67,14 @@ Also see: [`addBlobentry!`](@ref), [`getBlob`](@ref), [`listBlobentries`](@ref)
 """
 function getBlobentry(var::AbstractDFGVariable, key::Symbol)
     if !hasBlobentry(var, key)
-        throw(
-            KeyError(
-                "No dataEntry label $(key) found in variable $(getLabel(var)). Available keys: $(keys(var.dataDict))",
-            ),
-        )
+        throw(LabelNotFoundError("Blobentry", key, collect(keys(var.dataDict))))
     end
     return var.dataDict[key]
 end
 
 function getBlobentry(var::VariableDFG, key::Symbol)
     if !hasBlobentry(var, key)
-        throw(KeyError(key))
+        throw(LabelNotFoundError("Blobentry", key))
     end
     return var.blobEntries[findfirst(x -> x.label == key, var.blobEntries)]
 end
@@ -156,19 +152,18 @@ Should be extended if DFG variable is not returned by reference.
 
 Also see: [`getBlobentry`](@ref), [`addBlob!`](@ref), [`mergeBlobentries!`](@ref)
 """
-function addBlobentry!(var::AbstractDFGVariable, entry::Blobentry;)
+function addBlobentry!(var::AbstractDFGVariable, entry::Blobentry)
     # see https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/985
     # blobId::Union{UUID,Nothing} = (isnothing(entry.blobId) ? entry.id : entry.blobId),
     # blobSize::Int = (hasfield(Blobentry, :size) ? entry.size : -1)
-    haskey(var.dataDict, entry.label) &&
-        error("blobEntry $(entry.label) already exists on variable $(getLabel(var))")
+    haskey(var.dataDict, entry.label) && throw(LabelExistsError("Blobentry", entry.label))
     var.dataDict[entry.label] = entry
     return entry
 end
 
 function addBlobentry!(var::VariableDFG, entry::Blobentry)
     entry.label in getproperty.(var.blobEntries, :label) &&
-        error("blobEntry $(entry.label) already exists on variable $(getLabel(var))")
+        throw(LabelExistsError("Blobentry", entry.label))
     push!(var.blobEntries, entry)
     return entry
 end

src/Deprecated.jl

@@ -420,6 +420,16 @@ function fncStringToData(args...; kwargs...)
     return error("fncStringToData is obsolete.")
 end
 
+#TODO make sure getFactorOperationalMemoryType is obsolete
+function getFactorOperationalMemoryType(dummy)
+    return error(
+        "Please extend your workspace with function getFactorOperationalMemoryType(<:AbstractParams) for your usecase, e.g. IncrementalInference uses `CommonConvWrapper <: FactorSolverCache`",
+    )
+end
+function getFactorOperationalMemoryType(dfg::AbstractDFG)
+    return getFactorOperationalMemoryType(getSolverParams(dfg))
+end
+
 ## ================================================================================
 ## Deprecated in v0.25
 ##=================================================================================

src/DistributedFactorGraphs.jl

@@ -354,6 +354,7 @@ export plotDFG
 ##==============================================================================
 
 # Entities
+include("errors.jl")
 
 include("entities/AbstractDFG.jl")
 

src/GraphsDFG/FactorGraphs/FactorGraphs.jl

@@ -139,9 +139,10 @@ function addFactor!(
         (@error "Label $(factor.label) already in fg"; return false)
 
     for vlabel in variableLabels
-        !haskey(g.labels, vlabel) && (throw(KeyError(vlabel))
-        # @error "Variable '$(vlabel)' not found in graph when creating Factor '$(factor.label)'"; return false
-        ) #TODO debug error or exception?
+        if !haskey(g.labels, vlabel)
+            @error "Variable '$(vlabel)' not found in graph when creating Factor '$(factor.label)'"
+            return false
+        end
     end
 
     add_vertex!(g.graph) || return false

src/GraphsDFG/GraphsDFG.jl

@@ -8,7 +8,7 @@ using OrderedCollections
 using StructTypes
 
 using ...DistributedFactorGraphs
-using ...DistributedFactorGraphs: Agent
+using ...DistributedFactorGraphs: Agent, LabelNotFoundError, LabelExistsError
 
 # import DFG functions to extend
 import ...DistributedFactorGraphs:

src/GraphsDFG/services/GraphsDFG.jl

@@ -42,9 +42,8 @@ function addVariable!(
     dfg::GraphsDFG{<:AbstractParams, V, <:AbstractDFGFactor},
     variable::V,
 ) where {V <: AbstractDFGVariable}
-    #TODO should this be an error
     if haskey(dfg.g.variables, variable.label)
-        error("Variable '$(variable.label)' already exists in the factor graph")
+        throw(LabelExistsError("Variable", variable.label))
     end
 
     FactorGraphs.addVariable!(dfg.g, variable) || return false
@@ -104,11 +103,15 @@ function addFactor!(
     factor::F,
 ) where {F <: AbstractDFGFactor}
     if haskey(dfg.g.factors, factor.label)
-        error("Factor '$(factor.label)' already exists in the factor graph")
+        throw(LabelExistsError("Factor", factor.label))
     end
     # TODO
     # @assert FactorGraphs.addFactor!(dfg.g, getVariableOrder(factor), factor)
-    @assert FactorGraphs.addFactor!(dfg.g, Symbol[factor._variableOrderSymbols...], factor)
+    variableLabels = Symbol[factor._variableOrderSymbols...]
+    for vlabel in variableLabels
+        !exists(dfg, vlabel) && throw(LabelNotFoundError("Variable", vlabel))
+    end
+    @assert FactorGraphs.addFactor!(dfg.g, variableLabels, factor)
     return factor
 end
 
@@ -121,16 +124,15 @@ end
 
 function getVariable(dfg::GraphsDFG, label::Symbol)
     if !haskey(dfg.g.variables, label)
-        error("Variable label '$(label)' does not exist in the factor graph")
+        throw(LabelNotFoundError("Variable", label))
     end
 
     return dfg.g.variables[label]
 end
 
 function getFactor(dfg::GraphsDFG, label::Symbol)
     if !haskey(dfg.g.factors, label)
-        #TODO throw a typed error
-        error("Factor label '$(label)' does not exist in the factor graph")
+        throw(LabelNotFoundError("Factor", label))
     end
     return dfg.g.factors[label]
 end
@@ -151,8 +153,8 @@ function mergeFactor!(dfg::GraphsDFG, factor::AbstractDFGFactor;)
         #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)
+        # deleteFactor!(dfg, factor.label)
+        # addFactor!(dfg, factor)
     else
         dfg.g.factors[factor.label] = factor
     end
@@ -162,7 +164,7 @@ end
 
 function deleteVariable!(dfg::GraphsDFG, label::Symbol)#::Tuple{AbstractDFGVariable, Vector{<:AbstractDFGFactor}}
     if !haskey(dfg.g.variables, label)
-        error("Variable label '$(label)' does not exist in the factor graph")
+        throw(LabelNotFoundError("Variable", label))
     end
 
     deleteNeighbors = true # reserved, orphaned factors are not supported at this time
@@ -175,7 +177,7 @@ end
 
 function deleteFactor!(dfg::GraphsDFG, label::Symbol; suppressGetFactor::Bool = false)
     if !haskey(dfg.g.factors, label)
-        error("Factor label '$(label)' does not exist in the factor graph")
+        throw(LabelNotFoundError("Factor", label))
     end
     rem_vertex!(dfg.g, dfg.g.labels[label])
     return 1
@@ -280,10 +282,7 @@ end
 function _isSolvable(dfg::GraphsDFG, label::Symbol, ready::Int)
     haskey(dfg.g.variables, label) && (return dfg.g.variables[label].solvable >= ready)
     haskey(dfg.g.factors, label) && (return dfg.g.factors[label].solvable >= ready)
-
-    #TODO should this be a breaking error?
-    @error "Node not in factor or variable"
-    return false
+    throw(LabelNotFoundError(label))
 end
 
 function listNeighbors(dfg::GraphsDFG, node::DFGNode; solvable::Int = 0)
@@ -292,7 +291,7 @@ end
 
 function listNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int = 0)
     if !exists(dfg, label)
-        error("Variable/factor with label '$(label)' does not exist in the factor graph")
+        throw(LabelNotFoundError(label))
     end
 
     neighbors_il = FactorGraphs.outneighbors(dfg.g, dfg.g.labels[label])
@@ -540,9 +539,7 @@ end
 
 function addGraphBlobentry!(fg::GraphsDFG, entry::Blobentry)
     if haskey(fg.graphBlobEntries, entry.label)
-        error(
-            "Blobentry '$(entry.label)' already exists in the factor graph's blob entries.",
-        )
+        throw(LabelExistsError("Blobentry", entry.label))
     end
     push!(fg.graphBlobEntries, entry.label => entry)
     return entry

src/errors.jl

@@ -0,0 +1,57 @@
+"""
+    LabelNotFoundError(label, available)
+
+Error thrown when a requested label is not found in the factor graph.
+"""
+struct LabelNotFoundError <: Exception
+    name::String
+    label::Symbol
+    available::Vector{Symbol}
+end
+
+LabelNotFoundError(name::String, label::Symbol) = LabelNotFoundError(name, label, Symbol[])
+LabelNotFoundError(label::Symbol) = LabelNotFoundError("Node", label, Symbol[])
+
+function Base.showerror(io::IO, ex::LabelNotFoundError)
+    print(io, "LabelNotFoundError: ", ex.name, " label '", ex.label, "' not found.")
+    if !isempty(ex.available)
+        println(io, " Available labels:")
+        show(io, ex.available)
+    end
+end
+
+"""
+    LabelExistsError(label)
+
+Error thrown when attempting to add a label that already exists in the collection.
+"""
+struct LabelExistsError <: Exception
+    name::String
+    label::Symbol
+end
+
+LabelExistsError(label::Symbol) = LabelExistsError("Node", label)
+
+function Base.showerror(io::IO, ex::LabelExistsError)
+    return print(
+        io,
+        "LabelExistsError: ",
+        ex.name,
+        " label '",
+        ex.label,
+        "' already exists.",
+    )
+end
+
+"""
+    SerializationError(msg)
+
+Error thrown when serialization or deserialization fails.
+"""
+struct SerializationError <: Exception
+    msg::String
+end
+
+function Base.showerror(io::IO, ex::SerializationError)
+    return print(io, "SerializationError: ", ex.msg)
+end

src/services/AbstractDFG.jl

@@ -97,20 +97,6 @@ getAddHistory(dfg::AbstractDFG) = dfg.addHistory
 """
 getSolverParams(dfg::AbstractDFG) = dfg.solverParams
 
-"""
-    $(SIGNATURES)
-
-Method must be overloaded by the user for Serialization to work.  E.g. IncrementalInference uses `CommonConvWrapper <: FactorSolverCache`.
-"""
-function getFactorOperationalMemoryType(dummy)
-    return error(
-        "Please extend your workspace with function getFactorOperationalMemoryType(<:AbstractParams) for your usecase, e.g. IncrementalInference uses `CommonConvWrapper <: FactorSolverCache`",
-    )
-end
-function getFactorOperationalMemoryType(dfg::AbstractDFG)
-    return getFactorOperationalMemoryType(getSolverParams(dfg))
-end
-
 """
     $(SIGNATURES)
 
@@ -121,7 +107,10 @@ function rebuildFactorCache!(
     factor::AbstractDFGFactor,
     neighbors = [],
 )
-    @warn("rebuildFactorCache! is not implemented for $(typeof(dfg))")
+    @warn(
+        "FactorCache not build, rebuildFactorCache! is not implemented for $(typeof(dfg)). Make sure to load IncrementalInference.",
+        maxlog = 1
+    )
     return nothing
 end
 
@@ -533,7 +522,7 @@ function getVariable(dfg::AbstractDFG, label::Symbol, solveKey::Symbol)
     var = getVariable(dfg, label)
 
     if isa(var, VariableCompute) && !haskey(var.solverDataDict, solveKey)
-        error("Solvekey '$solveKey' does not exists in the variable")
+        throw(LabelNotFoundError("VariableNode", solveKey))
     elseif !isa(var, VariableCompute)
         @warn "getVariable(dfg, label, solveKey) only supported for type VariableCompute."
     end
@@ -1096,7 +1085,7 @@ function copyGraph!(
         elseif overwriteDest
             mergeVariable!(destDFG, variableCopy)
         else
-            error("Variable $(variable.label) already exists in destination graph!")
+            throw(LabelExistsError("Variable", variable.label))
         end
     end
     # And then all factors to the destDFG.