Refactor graph copy and subgraph functions

src/DFGPlots/DFGPlots.jl

@@ -88,7 +88,7 @@ More information at [GraphPlot.jl](https://github.com/JuliaGraphs/GraphPlot.jl)
 function dfgplot(dfg::AbstractDFG, p::DFGPlotProps = DFGPlotProps())
     # TODO implement convert functions
     ldfg = LightDFG{NoSolverParams}()
-    DistributedFactorGraphs._copyIntoGraph!(dfg, ldfg, union(listVariables(dfg), listFactors(dfg)), true, copyGraphMetadata=false)
+    copyGraph!(ldfg, dfg, union(listVariables(dfg), listFactors(dfg)), copyGraphMetadata=false)
     dfgplot(ldfg, p)
 end
 

src/Deprecated.jl

@@ -95,4 +95,199 @@ 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
+
+##==============================================================================
+## WIP on consolidated subgraph functions, aim to remove in 0.8
+##==============================================================================
+# Deprecate in favor of buildSubgraph, mergeGraph
+export getSubgraph, getSubgraphAroundNode
+export buildSubgraphFromLabels!
+
+"""
+    $(SIGNATURES)
+Common function for copying nodes from one graph into another graph.
+This is overridden in specialized implementations for performance.
+NOTE: copyGraphMetadata not supported yet.
+"""
+function _copyIntoGraph!(sourceDFG::G, destDFG::H, variableFactorLabels::Vector{Symbol}, includeOrphanFactors::Bool=false; copyGraphMetadata::Bool=false)::Nothing where {G <: AbstractDFG, H <: AbstractDFG}
+    # Split into variables and factors
+    Base.depwarn("_copyIntoGraph! is deprecated use copyGraph/deepcopyGraph[!]", :_copyIntoGraph!)
+
+    includeOrphanFactors && (@error "Adding orphaned factors is not supported")
+
+    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
+        if !exists(destDFG, variable)
+            addVariable!(destDFG, deepcopy(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, deepcopy(factor))
+            end
+        end
+    end
+
+    if copyGraphMetadata
+        setUserData(destDFG, getUserData(sourceDFG))
+        setRobotData(destDFG, getRobotData(sourceDFG))
+        setSessionData(destDFG, getSessionData(sourceDFG))
+    end
+    return nothing
+end
+
+"""
+    $(SIGNATURES)
+Retrieve a deep subgraph copy around a given variable or factor.
+Optionally provide a distance to specify the number of edges should be followed.
+Optionally provide an existing subgraph addToDFG, the extracted nodes will be copied into this graph. By default a new subgraph will be created.
+Note: By default orphaned factors (where the subgraph does not contain all the related variables) are not returned. Set includeOrphanFactors to return the orphans irrespective of whether the subgraph contains all the variables.
+Note: Always returns the node at the center, but filters around it if solvable is set.
+"""
+function getSubgraphAroundNode(dfg::AbstractDFG, node::DFGNode, distance::Int=1, includeOrphanFactors::Bool=false, addToDFG::AbstractDFG=_getDuplicatedEmptyDFG(dfg); solvable::Int=0)::AbstractDFG
+
+    Base.depwarn("getSubgraphAroundNode is deprecated use buildSubgraph", :getSubgraphAroundNode)
+
+    if !exists(dfg, node.label)
+        error("Variable/factor with label '$(node.label)' does not exist in the factor graph")
+    end
+
+    neighbors = getNeighborhood(dfg, node.label, distance)
+
+    # for some reason: always returns the node at the center with  || (nlbl == node.label)
+    solvable != 0 && filter!(nlbl -> (getSolvable(dfg, nlbl) >= solvable) || (nlbl == node.label), neighbors)
+
+    # Copy the section of graph we want
+    _copyIntoGraph!(dfg, addToDFG, neighbors, includeOrphanFactors)
+    return addToDFG
+end
+
+"""
+    $(SIGNATURES)
+Get a deep subgraph copy from the DFG given a list of variables and factors.
+Optionally provide an existing subgraph addToDFG, the extracted nodes will be copied into this graph. By default a new subgraph will be created.
+Note: By default orphaned factors (where the subgraph does not contain all the related variables) are not returned. Set includeOrphanFactors to return the orphans irrespective of whether the subgraph contains all the variables.
+"""
+function getSubgraph(dfg::G,
+                     variableFactorLabels::Vector{Symbol},
+                     includeOrphanFactors::Bool=false,
+                     addToDFG::H=_getDuplicatedEmptyDFG(dfg))::H where {G <: AbstractDFG, H <: AbstractDFG}
+
+    Base.depwarn("getSubgraph is deprecated use buildSubgraph", :getSubgraph)
+
+    for label in variableFactorLabels
+        if !exists(dfg, label)
+            error("Variable/factor with label '$(label)' does not exist in the factor graph")
+        end
+    end
+
+    _copyIntoGraph!(dfg, addToDFG, variableFactorLabels, includeOrphanFactors)
+    return addToDFG
+end
+
+
+# TODO needsahome: home should be in IIF, calling just deepcopyGraph, or copyGraph
+#                  Into, Labels, Subgraph are all implied from the parameters.
+#                  can alies names but like Sam suggested only on copy is needed.
+
+
+"""
+    $SIGNATURES
+Construct a new factor graph object as a subgraph of `dfg <: AbstractDFG` based on the
+variable labels `syms::Vector{Symbols}`.
+
+SamC: Can we not just use _copyIntoGraph! for this? Looks like a small refactor to make it work.
+Will paste in as-is for now and we can figure it out as we go.
+  DF: Absolutely agree that subgraph functions should use `DFG._copyIntoGraph!` as a single dependency in the code base.  There have been a repeated new rewrites of IIF.buildSubGraphFromLabels (basic wrapper is fine) but nominal should be to NOT duplicate DFG functionality in IIF -- rather use/improve the existing features in DFG. FYI, I have repeatedly refactored this function over and over to use DFG properly but somehow this (add/delete/Variable/Factor) version keeps coming back without using `_copyIntoGraph`!!???  See latest effort commented out below `buildSubgraphFromLabels!_SPECIAL`...
+
+Notes
+- Slighly messy internals, but gets the job done -- some room for performance improvement.
+- Defaults to GraphDFG, but likely to change to LightDFG in future.
+  - since DFG v0.6 LightDFG is the default.
+
+DevNotes
+- TODO: still needs to be consolidated with `DFG._copyIntoGraph`
+
+Related
+
+listVariables, _copyIntoGraph!
+"""
+function buildSubgraphFromLabels!(dfg::G,
+                                  syms::Vector{Symbol};
+                                  subfg::AbstractDFG=(G <: InMemoryDFGTypes ? G : GraphsDFG)(params=getSolverParams(dfg)),
+                                  solvable::Int=0,
+                                  allowedFactors::Union{Nothing, Vector{Symbol}}=nothing  )::AbstractDFG where G <: AbstractDFG
+  #
+  Base.depwarn("buildSubgraphFromLabels! is deprecated use copyGraph, buildSubgraph or buildCliqueSubgraph!(IIF)", :buildSubgraphFromLabels!)
+  # add a little too many variables (since we need the factors)
+  for sym in syms
+    if solvable <= getSolvable(dfg, sym)
+      getSubgraphAroundNode(dfg, getVariable(dfg, sym), 2, false, subfg, solvable=solvable)
+    end
+  end
+
+  # remove excessive variables that were copied by neighbors distance 2
+  currVars = listVariables(subfg)
+  toDelVars = setdiff(currVars, syms)
+  for dv in toDelVars
+    # delete any neighboring factors first
+    for fc in lsf(subfg, dv)
+      deleteFactor!(subfg, fc)
+    end
+
+    # and the variable itself
+    deleteVariable!(subfg, dv)
+  end
+
+  # delete any factors not in the allowed list
+  if allowedFactors != nothing
+    delFcts = setdiff(lsf(subfg), allowedFactors)
+    for dfct in delFcts
+      deleteFactor!(subfg, dfct)
+    end
+  end
+
+  # orphaned variables are allowed, but not orphaned factors
+
+  return subfg
+end

src/DistributedFactorGraphs.jl

@@ -161,8 +161,8 @@ export packVariableNodeData, unpackVariableNodeData
 
 export getSolvedCount, isSolved, setSolvedCount!, isInitialized
 
-export getNeighborhood, getSubgraph, getSubgraphAroundNode, getNeighbors, _getDuplicatedEmptyDFG
-
+export getNeighborhood, getNeighbors, _getDuplicatedEmptyDFG
+export copyGraph!, deepcopyGraph, deepcopyGraph!, buildSubgraph, mergeGraph!
 # Big Data
 ##------------------------------------------------------------------------------
 export addBigDataEntry!, getBigDataEntry, updateBigDataEntry!, deleteBigDataEntry!, getBigDataEntries, getBigDataKeys
@@ -224,13 +224,10 @@ export
     compareFactorGraphs
 
 
-## Deprecated.jl should be listed there
+## Deprecated exports should be listed in Deprecated.jl if possible, otherwise here 
 
 
 ## needsahome.jl
-
-export buildSubgraphFromLabels!
-
 import Base: print
 export printFactor, printVariable, print
 

src/GraphsDFG/services/GraphsDFG.jl

@@ -208,7 +208,7 @@ function getFactors(dfg::GraphsDFG, regexFilter::Union{Nothing, Regex}=nothing;
     if regexFilter != nothing
         factors = filter(f -> occursin(regexFilter, String(f.label)), factors)
     end
-    
+
     if length(tags) > 0
         mask = map(v -> length(intersect(v.tags, tags)) > 0, factors )
         return factors[mask]
@@ -255,42 +255,6 @@ function getNeighbors(dfg::GraphsDFG, label::Symbol; solvable::Int=0)::Vector{Sy
     return map(n -> n.dfgNode.label, neighbors)
 end
 
-#NOTE Replaced by abstract function in services/AbstractDFG.jl
-# """
-#     $(SIGNATURES)
-# Retrieve a deep subgraph copy around a given variable or factor.
-# Optionally provide a distance to specify the number of edges should be followed.
-# Optionally provide an existing subgraph addToDFG, the extracted nodes will be copied into this graph. By default a new subgraph will be created.
-# Note: By default orphaned factors (where the subgraph does not contain all the related variables) are not returned. Set includeOrphanFactors to return the orphans irrespective of whether the subgraph contains all the variables.
-# """
-# function getSubgraphAroundNode(dfg::GraphsDFG{P}, node::T, distance::Int64=1, includeOrphanFactors::Bool=false, addToDFG::GraphsDFG=GraphsDFG{P}(); solvable::Int=0)::GraphsDFG where {P <: AbstractParams, T <: DFGNode}
-#     if !haskey(dfg.labelDict, node.label)
-#         error("Variable/factor with label '$(node.label)' does not exist in the factor graph")
-#     end
-#
-#     # Build a list of all unique neighbors inside 'distance'
-#     neighborList = Dict{Symbol, Any}()
-#     push!(neighborList, node.label => dfg.g.vertices[dfg.labelDict[node.label]])
-#     curList = Dict{Symbol, Any}(node.label => dfg.g.vertices[dfg.labelDict[node.label]])
-#     for dist in 1:distance
-#         newNeighbors = Dict{Symbol, Any}()
-#         for (key, node) in curList
-#             neighbors = in_neighbors(node, dfg.g) #Don't use out_neighbors! It enforces directiveness even if we don't want it
-#             for neighbor in neighbors
-#                 if !haskey(neighborList, neighbor.dfgNode.label) && (isSolvable(neighbor.dfgNode) >= solvable)
-#                     push!(neighborList, neighbor.dfgNode.label => neighbor)
-#                     push!(newNeighbors, neighbor.dfgNode.label => neighbor)
-#                 end
-#             end
-#         end
-#         curList = newNeighbors
-#     end
-#
-#     # Copy the section of graph we want
-#     _copyIntoGraph!(dfg, addToDFG, collect(keys(neighborList)), includeOrphanFactors)
-#     return addToDFG
-# end
-
 """
     $(SIGNATURES)
 Produces a dot-format of the graph for visualization.

src/LightDFG/LightDFG.jl

@@ -3,7 +3,9 @@ module LightDFGs
 using LightGraphs
 using DocStringExtensions
 
-import ...DistributedFactorGraphs: AbstractDFG, DFGNode, AbstractDFGVariable, AbstractDFGFactor, DFGSummary, AbstractParams, NoSolverParams, DFGVariable, DFGFactor
+using ...DistributedFactorGraphs
+
+# import ...DistributedFactorGraphs: AbstractDFG, DFGNode, AbstractDFGVariable, AbstractDFGFactor, DFGSummary, AbstractParams, NoSolverParams, DFGVariable, DFGFactor
 
 # import DFG functions to extend
 import ...DistributedFactorGraphs:  setSolverParams!,
@@ -38,8 +40,8 @@ import ...DistributedFactorGraphs:  setSolverParams!,
                                     isFullyConnected,
                                     hasOrphans,
                                     getNeighbors,
-                                    getSubgraphAroundNode,
-                                    getSubgraph,
+                                    buildSubgraph,
+                                    copyGraph!,
                                     getBiadjacencyMatrix,
                                     _getDuplicatedEmptyDFG,
                                     toDot,

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)