Refactor graph copy and subgraph functions

src/Deprecated.jl

@@ -95,4 +95,23 @@ Base.setproperty!(x::DFGFactor,f::Symbol, val) = begin
     end
 
 
-#
+#NOTE deprecate in favor of constructors because its not lossless: https://docs.julialang.org/en/v1/manual/conversion-and-promotion/#Conversion-vs.-Construction-1
+function Base.convert(::Type{DFGVariableSummary}, v::DFGVariable)
+    Base.depwarn("convert to type DFGVariableSummary is deprecated use the constructor", :convert)
+    return DFGVariableSummary(v)
+end
+
+function Base.convert(::Type{SkeletonDFGVariable}, v::VariableDataLevel1)
+    Base.depwarn("convert to type SkeletonDFGVariable is deprecated use the constructor", :convert)
+    return SkeletonDFGVariable(v)
+end
+
+function Base.convert(::Type{DFGFactorSummary}, f::DFGFactor)
+    Base.depwarn("convert to type DFGFactorSummary is deprecated use the constructor", :convert)
+    return DFGFactorSummary(f)
+end
+
+function Base.convert(::Type{SkeletonDFGFactor}, f::FactorDataLevel1)
+    Base.depwarn("convert to type SkeletonDFGFactor is deprecated use the constructor", :convert)
+    return SkeletonDFGFactor(f)
+end

src/DistributedFactorGraphs.jl

@@ -161,8 +161,10 @@ export packVariableNodeData, unpackVariableNodeData
 
 export getSolvedCount, isSolved, setSolvedCount!, isInitialized
 
-export getNeighborhood, getSubgraph, getSubgraphAroundNode, getNeighbors, _getDuplicatedEmptyDFG
-
+export getNeighborhood, getNeighbors, _getDuplicatedEmptyDFG
+export buildSubgraph
+# TODO Deprecate in favor of buildSubgraph
+export getSubgraph, getSubgraphAroundNode
 # Big Data
 ##------------------------------------------------------------------------------
 export addBigDataEntry!, getBigDataEntry, updateBigDataEntry!, deleteBigDataEntry!, getBigDataEntries, getBigDataKeys
@@ -250,6 +252,8 @@ include("entities/AbstractDFGSummary.jl")
 
 include("services/AbstractDFG.jl")
 
+include("services/copyFunctions.jl")
+
 # In Memory Types
 include("GraphsDFG/GraphsDFG.jl")
 include("LightDFG/LightDFG.jl")

src/LightDFG/entities/LightDFG.jl

@@ -66,3 +66,14 @@ LightDFG(description::String,
          sessionData::Dict{Symbol, String},
          solverParams::AbstractParams) =
          LightDFG(FactorGraph{Int,DFGVariable,DFGFactor}(), description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], solverParams)
+
+
+LightDFG{T,V,F}(description::String,
+                userId::String,
+                robotId::String,
+                sessionId::String,
+                userData::Dict{Symbol, String},
+                robotData::Dict{Symbol, String},
+                sessionData::Dict{Symbol, String},
+                solverParams::T) where {T <: AbstractParams, V <:AbstractDFGVariable, F<:AbstractDFGFactor} =
+                LightDFG(FactorGraph{Int,V,F}(), description, userId, robotId, sessionId, userData, robotData, sessionData, Symbol[], solverParams)

src/LightDFG/services/LightDFG.jl

@@ -38,6 +38,12 @@ function addVariable!(dfg::LightDFG{<:AbstractParams, V, <:AbstractDFGFactor}, v
     return variable
 end
 
+function addVariable!(dfg::LightDFG{<:AbstractParams, VD, <:AbstractDFGFactor},
+                      variable::AbstractDFGVariable)::VD where VD <: AbstractDFGVariable
+    return addVariable!(dfg, VD(variable))
+end
+
+
 #moved to abstract
 # function addFactor!(dfg::LightDFG{<:AbstractParams, V, F}, variables::Vector{<:V}, factor::F)::F where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
 #
@@ -86,6 +92,11 @@ function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F}, f
     return factor
 end
 
+function addFactor!(dfg::LightDFG{<:AbstractParams, <:AbstractDFGVariable, F},
+                      factor::AbstractDFGFactor)::F where F <: AbstractDFGFactor
+    return addFactor!(dfg, F(factor))
+end
+
 function getVariable(dfg::LightDFG, label::Symbol)::AbstractDFGVariable
     if !haskey(dfg.g.variables, label)
         error("Variable label '$(label)' does not exist in the factor graph")
@@ -257,7 +268,7 @@ end
 function _copyIntoGraph!(sourceDFG::LightDFG{<:AbstractParams, V, F}, destDFG::LightDFG{<:AbstractParams, V, F}, ns::Vector{Int}, includeOrphanFactors::Bool=false)::Nothing where {V <: AbstractDFGVariable, F <: AbstractDFGFactor}
 
     includeOrphanFactors && (@error "Adding orphaned factors is not supported")
-    
+
     #kan ek die in bulk copy, soos graph en dan nuwe map maak
     # Add all variables first,
     labels = [sourceDFG.g.labels[i] for i in ns]

src/entities/DFGFactor.jl

@@ -183,13 +183,11 @@ const FactorDataLevel1 = Union{DFGFactor, DFGFactorSummary}
 const FactorDataLevel2 = Union{DFGFactor}
 
 ##==============================================================================
-## Convert
+## Convertion constructors
 ##==============================================================================
-function Base.convert(::Type{DFGFactorSummary}, f::DFGFactor)
-    return DFGFactorSummary(f.label, f.timestamp, deepcopy(f.tags), f._dfgNodeParams._internalId, deepcopy(f._variableOrderSymbols))
-end
 
-#TODO TEST
-function Base.convert(::Type{SkeletonDFGFactor}, f::FactorDataLevel1)
-    return SkeletonDFGFactor(f.label, deepcopy(f.tags), deepcopy(f._variableOrderSymbols))
-end
+DFGFactorSummary(f::DFGFactor) =
+    DFGFactorSummary(f.label, f.timestamp, deepcopy(f.tags), f._dfgNodeParams._internalId, deepcopy(f._variableOrderSymbols))
+
+SkeletonDFGFactor(f::FactorDataLevel1) =
+    SkeletonDFGFactor(f.label, deepcopy(f.tags), deepcopy(f._variableOrderSymbols))

src/entities/DFGVariable.jl

@@ -309,14 +309,11 @@ const VariableDataLevel1 = Union{DFGVariable, DFGVariableSummary}
 const VariableDataLevel2 = Union{DFGVariable}
 
 ##==============================================================================
-## converters
+## Convertion constructors
 ##==============================================================================
 
-function Base.convert(::Type{DFGVariableSummary}, v::DFGVariable)
-    return DFGVariableSummary(v.label, v.timestamp, deepcopy(v.tags), deepcopy(v.ppeDict), Symbol(typeof(getSofttype(v))), v.bigData, v._internalId)
-end
+DFGVariableSummary(v::DFGVariable) =
+        DFGVariableSummary(v.label, v.timestamp, deepcopy(v.tags), deepcopy(v.ppeDict), Symbol(typeof(getSofttype(v))), v.bigData, v._internalId)
 
-#TODO Test
-function Base.convert(::Type{SkeletonDFGVariable}, v::VariableDataLevel1)
-    return SkeletonDFGVariable(v.label, deepcopy(v.tags))
-end
+SkeletonDFGVariable(v::VariableDataLevel1) =
+            SkeletonDFGVariable(v.label, deepcopy(v.tags))

src/services/AbstractDFG.jl

@@ -848,6 +848,27 @@ function getSubgraph(dfg::G,
     return addToDFG
 end
 
+
+function buildSubgraph(::Type{G}, dfg::AbstractDFG, variableFactorLabels::Vector{Symbol}, distance::Int=0; solvable::Int=0, kwargs...) where G <: AbstractDFG
+
+    # find neighbors at distance to add
+    neighbors = Symbol[]
+    if distance > 0
+        for l in variableFactorLabels
+            union!(neighbors, getNeighborhood(dfg, l, distance))
+        end
+    end
+
+    allvarfacs = union(variableFactorLabels, neighbors)
+
+    solvable != 0 && filter!(nlbl -> (getSolvable(dfg, nlbl) >= solvable), allvarfacs)
+
+    # Copy the section of graph we want
+    destDFG = deepcopyGraph(G, dfg, allvarfacs; kwargs...)
+    return destDFG
+end
+
+
 """
     $(SIGNATURES)
 Common function for copying nodes from one graph into another graph.
@@ -1004,7 +1025,6 @@ function toDot(dfg::AbstractDFG)::String
     #TODO implement convert
     graphsdfg = GraphsDFG{NoSolverParams}()
     DistributedFactorGraphs._copyIntoGraph!(dfg, graphsdfg, union(listVariables(dfg), listFactors(dfg)), true)
-
     # Calls down to GraphsDFG.toDot
     return toDot(graphsdfg)
 end
@@ -1044,8 +1064,8 @@ Get a summary of the graph (first-class citizens of variables and factors).
 Returns a DFGSummary.
 """
 function getSummary(dfg::G)::DFGSummary where {G <: AbstractDFG}
-    vars = map(v -> convert(DFGVariableSummary, v), getVariables(dfg))
-    facts = map(f -> convert(DFGFactorSummary, f), getFactors(dfg))
+    vars = map(v -> DFGVariableSummary(v), getVariables(dfg))
+    facts = map(f -> DFGFactorSummary(f), getFactors(dfg))
     return DFGSummary(
         Dict(map(v->v.label, vars) .=> vars),
         Dict(map(f->f.label, facts) .=> facts),
@@ -1066,11 +1086,12 @@ function getSummaryGraph(dfg::G)::LightDFG{NoSolverParams, DFGVariableSummary, D
         userId=dfg.userId,
         robotId=dfg.robotId,
         sessionId=dfg.sessionId)
-    for v in getVariables(dfg)
-        newV = addVariable!(summaryDfg, convert(DFGVariableSummary, v))
-    end
-    for f in getFactors(dfg)
-        addFactor!(summaryDfg, getNeighbors(dfg, f), convert(DFGFactorSummary, f))
-    end
+    deepcopyGraph!(summaryDfg, dfg)
+    # for v in getVariables(dfg)
+    #     newV = addVariable!(summaryDfg, DFGVariableSummary(v))
+    # end
+    # for f in getFactors(dfg)
+    #     addFactor!(summaryDfg, getNeighbors(dfg, f), DFGFactorSummary(f))
+    # end
     return summaryDfg
 end

src/services/copyFunctions.jl

@@ -0,0 +1,79 @@
+"""
+    $(SIGNATURES)
+Common function for copying nodes from one graph into another graph.
+This is overridden in specialized implementations for performance.
+Orphaned factors are not added.
+Set `overwriteDest` to overwrite existing variables and factors in the destination DFG.
+NOTE: copyGraphMetadata not supported yet.
+"""
+function copyGraph!(destDFG::AbstractDFG, sourceDFG::AbstractDFG, variableFactorLabels::Vector{Symbol}; copyGraphMetadata::Bool=false, overwriteDest::Bool=false, deepcopyNodes::Bool=false)
+    # Split into variables and factors
+    sourceVariables = map(vId->getVariable(sourceDFG, vId), intersect(listVariables(sourceDFG), variableFactorLabels))
+    sourceFactors = map(fId->getFactor(sourceDFG, fId), intersect(listFactors(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
+        variableCopy = deepcopyNodes ? deepcopy(variable) : variable
+        if !exists(destDFG, variable)
+            addVariable!(destDFG, variableCopy)
+        elseif overwriteDest
+            updateVariable!(destDFG, variableCopy)
+        else
+            error("Variable $(variable.label) already exists in destination graph!")
+        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 length(factVariableIds) == length(sourceFactorVariableIds)
+            factorCopy = deepcopyNodes ? deepcopy(factor) : factor
+            if !exists(destDFG, factor)
+                addFactor!(destDFG, factorCopy)
+            elseif overwriteDest
+                updateFactor!(destDFG, factorCopy)
+            else
+                error("Factor $(factor.label) already exists in destination graph!")
+            end
+        else
+            @warn "Factor $(factor.label) will be an orphan in the destination graph, and therefore not added."
+        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} = union(ls(sourceDFG), lsf(sourceDFG));
+                        kwargs...)
+    copyGraph!(destDFG, sourceDFG, variableFactorLabels; deepcopyNodes=true, kwargs...)
+end
+
+
+function deepcopyGraph( ::Type{T},
+                        sourceDFG::AbstractDFG,
+                        variableFactorLabels::Vector{Symbol} = union(ls(sourceDFG), lsf(sourceDFG));
+                        kwargs...) where T <: AbstractDFG
+    destDFG = T(getDFGInfo(sourceDFG)...)
+    copyGraph!(destDFG, sourceDFG, variableFactorLabels; deepcopyNodes=true, kwargs...)
+    return destDFG
+end

test/interfaceTests.jl

@@ -70,6 +70,10 @@ end
     GettingSubgraphs(testDFGAPI)
 end
 
+@testset "Building Subgraphs" begin
+    BuildingSubgraphs(testDFGAPI)
+end
+
 #TODO Summaries and Summary Graphs
 @testset "Summaries and Summary Graphs" begin
     Summaries(testDFGAPI)
@@ -94,8 +98,13 @@ end
 
     fgcopy = testDFGAPI()
     DFG._copyIntoGraph!(fg, fgcopy, union(ls(fg), lsf(fg)))
+    @test getVariableOrder(fg,:f1) == getVariableOrder(fgcopy,:f1)
 
+    #test copyGraph, deepcopyGraph[!]
+    fgcopy = testDFGAPI()
+    DFG.deepcopyGraph!(fgcopy, fg)
     @test getVariableOrder(fg,:f1) == getVariableOrder(fgcopy,:f1)
 
+    CopyFunctionsTest(testDFGAPI)
 
 end