Some Progress on tests, bit of a mess still

Author: Affie

src/Deprecated.jl

@@ -9,6 +9,10 @@ Base.getproperty(x::VariableDataLevel1,f::Symbol) = begin
     if f == :estimateDict
       @warn "estimateDict is deprecated, use ppeDict instead"
       getfield(x, :ppeDict)
+    elseif f == :solvable
+      getfield(x,:_dfgNodeParams).solvable
+    elseif f == :_internalId
+      getfield(x,:_dfgNodeParams)._internalId
     else
       getfield(x,f)
     end

src/DistributedFactorGraphs.jl

@@ -68,7 +68,7 @@ export getVariableOrder
 
 # Services/AbstractDFG Exports
 export isInitialized, getFactorFunction, isVariable, isFactor
-export isSolvable, isSolveInProgress, getSolvable, setSolvable!
+export isSolvable, isSolveInProgress, getSolvable, setSolvable!, getSolveInProgress
 export mergeUpdateVariableSolverData!, mergeUpdateGraphSolverData!
 
 # Solver (IIF) Exports

src/LightDFG/entities/LightDFG.jl

@@ -50,21 +50,26 @@ Create an in-memory LightDFG with the following parameters:
 - V: Variable type
 - F: Factor type
 """
-LightDFG{T}(g::FactorGraph{Int,DFGVariable,DFGFactor}=FactorGraph{Int,DFGVariable,DFGFactor}(); kwargs...) where T <: AbstractParams = LightDFG{T,DFGVariable,DFGFactor}(g; kwargs...)
+LightDFG{T}(g::FactorGraph{Int,DFGVariable,DFGFactor}=FactorGraph{Int,DFGVariable,DFGFactor}(); kwargs...) where T <: AbstractParams =
+        LightDFG{T,DFGVariable,DFGFactor}(g; kwargs...)
 
-Base.propertynames(x::LightDFG, private::Bool=false) =
-    (:g, :description, :userId, :robotId, :sessionId, :nodeCounter, :labelDict, :addHistory, :solverParams)
-        # (private ? fieldnames(typeof(x)) : ())...)
+LightDFG(g::FactorGraph{Int,DFGVariable,DFGFactor}=FactorGraph{Int,DFGVariable,DFGFactor}(); params::T=NoSolverParams(), kwargs...)  where T =
+        LightDFG{T,DFGVariable,DFGFactor}(g; params=params, kwargs...)
 
-Base.getproperty(x::LightDFG,f::Symbol) = begin
-    if f == :nodeCounter
-        @error "Field nodeCounter deprecated. returning number of nodes"
-        nv(x.g)
-    elseif f == :labelDict
-        @error "Field labelDict deprecated. Consider using exists(dfg,label) or getLabelDict(dfg) instead. Returning internals copy"
-        #TODO: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/111
-        copy(x.g.labels.sym_int)
-    else
-        getfield(x,f)
-    end
-end
+# Fully depcrecate nodeCounter and labelDict
+# Base.propertynames(x::LightDFG, private::Bool=false) =
+#     (:g, :description, :userId, :robotId, :sessionId, :nodeCounter, :labelDict, :addHistory, :solverParams)
+#         # (private ? fieldnames(typeof(x)) : ())...)
+#
+# Base.getproperty(x::LightDFG,f::Symbol) = begin
+#     if f == :nodeCounter
+#         @error "Field nodeCounter deprecated. returning number of nodes"
+#         nv(x.g)
+#     elseif f == :labelDict
+#         @error "Field labelDict deprecated. Consider using exists(dfg,label) or getLabelDict(dfg) instead. Returning internals copy"
+#         #TODO: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/111
+#         copy(x.g.labels.sym_int)
+#     else
+#         getfield(x,f)
+#     end
+# end

src/LightDFG/services/LightDFG.jl

@@ -67,7 +67,7 @@ function addVariable!(dfg::LightDFG{<:AbstractParams, V, <:AbstractDFGFactor}, v
     return true
 end
 
-function addFactor!(dfg::LightDFG{<:AbstractParams, V, F}, variables::Vector{V}, factor::F)::Bool where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
+function addFactor!(dfg::LightDFG{<:AbstractParams, V, F}, variables::Vector{<:V}, factor::F)::Bool where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
     # if haskey(dfg.g.metaindex[:label], factor.label)
     #     error("Factor '$(factor.label)' already exists in the factor graph")
     # end
@@ -115,7 +115,7 @@ end
 
 #TODO Sam, I would say if we handle the update vsd corectly this function is only needed in the cloud.
 #   In memory types are just passing pointers along so no perfomrance issue.
-function getVariable(dfg::LightDFG, label::Symbol; solveKey::Union{Nothing, Symbol})::Union{Nothing, AbstractDFGVariable}
+function getVariable(dfg::LightDFG, label::Symbol; solveKey::Union{Nothing, Symbol}=nothing)::Union{Nothing, AbstractDFGVariable}
 
     if !haskey(dfg.g.variables, label)
         @warn "Variable '$label' does not exists in the factor graph"
@@ -393,7 +393,7 @@ end
 
 function getAdjacencyMatrixSparse(dfg::LightDFG; solvable::Int=0)
     @warn "Deprecated function, please use getBiadjacencyMatrixSparse as this will be removed in v0.6.1"
-    return getBiadjacencyMatrixSparse(dfg, solvable)
+    return getBiadjacencyMatrixSparse(dfg, solvable=solvable)
 end
 
 # this would be an incidence matrix

src/entities/DFGFactor.jl

@@ -47,7 +47,7 @@ mutable struct DFGFactor{T, S} <: AbstractDFGFactor
     timestamp::DateTime
     """Factor tags, e.g [:FACTOR].
     Accessors: `getTags`, `addTags!`, and `deleteTags!`"""
-    tags::Vector{Symbol}
+    tags::Set{Symbol}#Vector{Symbol} #TODO I propose we use a set since tags can't be repeated.
     """Solver data.
     Accessors: `getSolverData`, `setSolverData!`"""
     solverData::GenericFunctionNodeData{T, S}
@@ -69,8 +69,8 @@ $(SIGNATURES)
 Construct a DFG factor given a label.
 """
 #TODO _internalId?
-DFGFactor{T, S}(label::Symbol, _internalId::Int64=0, timestamp::DateTime=now()) where {T, S}
-            = DFGFactor(label, timestamp, Symbol[], GenericFunctionNodeData{T, S}(), 0, DFGNodeParams(0, _internalId), Symbol[])
+DFGFactor{T, S}(label::Symbol, internalId::Int64=0, timestamp::DateTime=now()) where {T, S} =
+                DFGFactor(label, timestamp, Symbol[], GenericFunctionNodeData{T, S}(), 0, DFGNodeParams(0, internalId), Symbol[])
 
 DFGFactor(label::Symbol;
     timestamp::DateTime=now(),

src/entities/DFGVariable.jl

@@ -20,7 +20,7 @@ struct DFGVariable{T<:InferenceVariable} <: AbstractDFGVariable
     timestamp::DateTime
     """Variable tags, e.g [:POSE, :VARIABLE, and :LANDMARK].
     Accessors: `getTags`, `addTags!`, and `deleteTags!`"""
-    tags::Vector{Symbol}
+    tags::tags::Set{Symbol}#Vector{Symbol} #TODO I propose we use a set since tags can't be repeated.
     """Dictionary of estimates keyed by solverDataDict keys
     Accessors: `addEstimate!`, `updateEstimate!`, and `deleteEstimate!`"""
     ppeDict::Dict{Symbol, <: AbstractPointParametricEst}

src/sandbox.jl

@@ -0,0 +1,65 @@
+
+function copyGraph!(destDFG::AbstractDFG, sourceDFG::AbstractDFG, variableFactorLabels::Vector{Symbol}, includeOrphanFactors::Bool=false; copyGraphMetadata::Bool=false)
+    # Split into variables and factors
+    sourceVariables = map(vId->getVariable(sourceDFG, vId), intersect(getVariableIds(sourceDFG), variableFactorLabels))
+    sourceFactors = map(fId->getFactor(sourceDFG, fId), intersect(getFactorIds(sourceDFG), variableFactorLabels))
+    if length(sourceVariables) + length(sourceFactors) != length(variableFactorLabels)
+        rem = symdiff(map(v->v.label, sourceVariables), variableFactorLabels)
+        rem = symdiff(map(f->f.label, sourceFactors), variableFactorLabels)
+        error("Cannot copy because cannot find the following nodes in the source graph: $rem")
+    end
+
+    # Now we have to add all variables first,
+    for variable in sourceVariables
+        if !exists(destDFG, variable)
+            addVariable!(destDFG, variable)
+        end
+    end
+    # And then all factors to the destDFG.
+    for factor in sourceFactors
+        # Get the original factor variables (we need them to create it)
+        sourceFactorVariableIds = getNeighbors(sourceDFG, factor)
+        # Find the labels and associated variables in our new subgraph
+        factVariableIds = Symbol[]
+        for variable in sourceFactorVariableIds
+            if exists(destDFG, variable)
+                push!(factVariableIds, variable)
+            end
+        end
+        # Only if we have all of them should we add it (otherwise strange things may happen on evaluation)
+        if includeOrphanFactors || length(factVariableIds) == length(sourceFactorVariableIds)
+            if !exists(destDFG, factor)
+                addFactor!(destDFG, factVariableIds, factor)
+            end
+        end
+    end
+
+    if copyGraphMetadata
+        setUserData(destDFG, getUserData(sourceDFG))
+        setRobotData(destDFG, getRobotData(sourceDFG))
+        setSessionData(destDFG, getSessionData(sourceDFG))
+    end
+    return nothing
+end
+
+function deepcopyGraph!(destDFG::AbstractDFG, sourceDFG::AbstractDFG, variableFactorLabels::Vector{Symbol}, includeOrphanFactors::Bool=false; copyGraphMetadata::Bool=false)
+    sourceDFGcopy = deepcopy(sourceDFG)
+    deepcopyGraph!(destDFG, sourceDFGcopy, union(ls(sourceDFG), lsf(sourceDFG)), includeOrphanFactors, copyGraphMetadata=copyGraphMetadata)
+end
+
+function deepcopyGraph!(destDFG::AbstractDFG, sourceDFG::AbstractDFG)
+    deepcopyGraph!(destDFG, sourceDFG, union(ls(sourceDFG), lsf(sourceDFG)), includeOrphanFactors, copyGraphMetadata=copyGraphMetadata)
+end
+
+function deepcopyGraph(::Type{T}, sourceDFG::AbstractDFG, variableFactorLabels::Vector{Symbol}, includeOrphanFactors::Bool=false; copyGraphMetadata::Bool=false) where T <: AbstractDFG
+    destDFG = T(getConstructionData(sourceDFG))
+    deepcopyGraph!(destDFG, sourceDFG, variableFactorLabels, includeOrphanFactors, copyGraphMetadata=copyGraphMetadata)
+    return destDFG
+end
+
+function deepcopyGraph(::Type{T}, sourceDFG::AbstractDFG) where T <: AbstractDFG
+    destDFG = T(getConstructionData(sourceDFG))
+    return destDFG
+end
+
+Base.convert(::Type{T}, fg::AbstractDFG) where T <: AbstractDFG = deepcopyGraph(T, fg)

src/services/AbstractDFG.jl

@@ -710,7 +710,7 @@ isSolvable
 """
 function getSolveInProgress(var::Union{DFGVariable, DFGFactor}; solveKey::Symbol=:default)::Int
     # Variable
-    var isa DFGVariable && return haskey(solverDataDict(var), solveKey) ? solverDataDict(var)[solveKey].solveInProgress : 0
+    var isa DFGVariable && return haskey(getSolverDataDict(var), solveKey) ? getSolverDataDict(var)[solveKey].solveInProgress : 0
     # Factor
     return solverData(var).solveInProgress
 end

test/interfaceTests.jl

@@ -1,4 +1,5 @@
-# global testDFGAPI = GraphsDFG
+#
+global testDFGAPI = LightDFG
 
 dfg = testDFGAPI{NoSolverParams}()
 
@@ -21,8 +22,8 @@ st2 = TestInferenceVariable2()
 solverData(v1).softtype = deepcopy(st1)
 solverData(v2).softtype = deepcopy(st2)
 
-# set v2 solvable
-setSolvable!(v2, 1)
+# set v1 to solvable=0
+setSolvable!(v1, 0)
 # set v1 and f1 solveInProgress
 solverData(v1).solveInProgress = 1
 solverData(f1).solveInProgress = 1
@@ -114,13 +115,14 @@ end
 # Gets
 @testset "Gets, Sets, and Accessors" begin
     global dfg,v1,v2,f1
+    #TODO compare variable and factor
     @test getVariable(dfg, v1.label) == v1
     @test getVariable(dfg, v2.label) != v1
     @test getFactor(dfg, f1.label) == f1
     f2 = deepcopy(f1)
     f2.label = :something
     @test f2 != f1
-    @test_throws Exception getVariable(dfg, :nope)
+    @test getVariable(dfg, :nope) == nothing
     @test_throws Exception getVariable(dfg, "nope")
     @test_throws Exception getFactor(dfg, :nope)
     @test_throws Exception getFactor(dfg, "nope")
@@ -145,6 +147,7 @@ end
 
     @test getTimestamp(v1) == v1.timestamp
     testTimestamp = now()
+    #TODO set timestamp immutable
     @test setTimestamp!(v1, testTimestamp) == testTimestamp
     @test getTimestamp(v1) == testTimestamp
 
@@ -158,9 +161,10 @@ end
     @test solverData(v1) === v1.solverDataDict[:default]
     @test getData(v1) === v1.solverDataDict[:default]
     @test solverData(v1, :default) === v1.solverDataDict[:default]
-    @test solverDataDict(v1) == v1.solverDataDict
-    @test internalId(v1) == v1._internalId
+    @test getSolverDataDict(v1) == v1.solverDataDict
+    @test getInternalId(v1) == v1._internalId
 
+    #TODO I don't know what is supposed to happen to softtype
     @test getSofttype(v1) == st1
     @test getSofttype(v2) == st2
     @test getSofttype(v1) == st1
@@ -170,12 +174,12 @@ end
     @test setTags!(v1, testTags) == testTags
     @test getTags(v1) == testTags
 
-    @test solverData(f1) == f1.data
+    @test solverData(f1) == f1.solverData
     # Deprecated functions
-    @test solverData(f1) == f1.data
-    @test getData(f1) == f1.data
+    @test solverData(f1) == f1.solverData
+    # REMOVED? @test getData(f1) == f1.data
     # Internal function
-    @test internalId(f1) == f1._internalId
+    @test getInternalId(f1) == f1._dfgNodeParams._internalId
 
     @test getSolverParams(dfg) != nothing
     @test setSolverParams(dfg, getSolverParams(dfg)) == getSolverParams(dfg)
@@ -188,8 +192,11 @@ end
     @test isSolvable(v1) == 0
     @test isSolvable(v2) == 1
     @test isSolvable(f1) == 0
-    @test isSolveInProgress(v1) == 1
-    @test isSolveInProgress(f1) == 1
+    @test getSolvable(v1) == 0
+
+    #TODO isSolveInProgress was not deprecated
+    @test getSolveInProgress(v1) == 1
+    @test getSolveInProgress(f1) == 1
     # setSolvable, getSolvable, and variants
     v1 = getVariable(dfg, v1.label)
     f1 = getFactor(dfg, f1.label)
@@ -280,7 +287,8 @@ end
     mergeUpdateVariableSolverData!(dfg, newvar)
 
     #For now spot check
-    @test solverDataDict(newvar) == solverDataDict(var)
+    #TODO solverDataDict() not deprecated
+    @test getSolverDataDict(newvar) == getSolverDataDict(var)
     @test getVariablePPEs(newvar) == getVariablePPEs(var)
     @test getMaxPPE(getVariablePPEs(newvar)[:default]) == getVariablePPEs(newvar)[:default].max
     @test getMeanPPE(getVariablePPEs(newvar)[:default]) == getVariablePPEs(newvar)[:default].mean
@@ -318,7 +326,8 @@ end
     @test symdiff(adjMat[1, :], [nothing, :a, :b, :orphan]) == Symbol[]
     @test symdiff(adjMat[2, :], [:f1, :f1, :f1, nothing]) == Symbol[]
     #sparse
-    adjMat, v_ll, f_ll = getIncidenceMatrixSparse(dfg)
+    #TODO this silly name thing has gone on too long
+    adjMat, v_ll, f_ll = DistributedFactorGraphs.getIncidenceMatrix(dfg)
     @test size(adjMat) == (1,3)
 
     # Checking the elements of adjacency, its not sorted so need indexing function
@@ -360,8 +369,9 @@ numNodes = 10
 dfg = testDFGAPI{NoSolverParams}()
 verts = map(n -> DFGVariable(Symbol("x$n"), TestInferenceVariable1()), 1:numNodes)
 #change ready and solveInProgress for x7,x8 for improved tests on x7x8f1
-verts[7].solvable = 1
-
+#NOTE because defaults changed
+verts[8]._dfgNodeParams.solvable = 0
+verts[9]._dfgNodeParams.solvable = 0
 # Can't change the softtypes now.
 # #force softytypes to first 2 vertices.
 # verts[1].solverDataDict[:default].softtype = deepcopy(st1)
@@ -370,6 +380,8 @@ verts[7].solvable = 1
 map(v -> addVariable!(dfg, v), verts)
 map(n -> addFactor!(dfg, [verts[n], verts[n+1]], DFGFactor{Int, :Symbol}(Symbol("x$(n)x$(n+1)f1"))), 1:(numNodes-1))
 
+getFactor(dfg, :x7x8f1)._dfgNodeParams.solvable = 0
+
 @testset "Getting Neighbors" begin
     global dfg,verts
     # Trivial test to validate that intersect([], []) returns order of first parameter
@@ -433,6 +445,7 @@ end
     end
 end
 
+#TODO Summaries and Summary Graphs
 @testset "Summaries and Summary Graphs" begin
     factorFields = fieldnames(DFGFactorSummary)
     variableFields = fieldnames(DFGVariableSummary)
@@ -471,7 +484,7 @@ end
     addVariable!(dotdfg, v1)
     addVariable!(dotdfg, v2)
     addFactor!(dotdfg, [v1, v2], f1)
-
+    #TODO hardcoded will have different results so test LightGraphs seperately
     @test toDot(dotdfg) == "graph graphname {\n2 [\"label\"=\"b\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n2 -- 3\n3 [\"label\"=\"f1\",\"shape\"=\"box\",\"fillcolor\"=\"blue\",\"color\"=\"blue\"]\n1 [\"label\"=\"a\",\"shape\"=\"ellipse\",\"fillcolor\"=\"red\",\"color\"=\"red\"]\n1 -- 3\n}\n"
     @test toDotFile(dotdfg, "something.dot") == nothing
     Base.rm("something.dot")