deprecate buildSubgraphFromLabels!, getSubgraph, getSubgraphAroundNode, _copyIntoGraph!

Author: Affie

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

@@ -115,3 +115,176 @@ 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
+##==============================================================================
+
+"""
+    $(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

@@ -162,7 +162,7 @@ export packVariableNodeData, unpackVariableNodeData
 export getSolvedCount, isSolved, setSolvedCount!, isInitialized
 
 export getNeighborhood, getNeighbors, _getDuplicatedEmptyDFG
-export buildSubgraph
+export copyGraph!, deepcopyGraph, deepcopyGraph!, buildSubgraph
 # TODO Deprecate in favor of buildSubgraph
 export getSubgraph, getSubgraphAroundNode
 # Big Data
@@ -252,8 +252,6 @@ include("entities/AbstractDFGSummary.jl")
 
 include("services/AbstractDFG.jl")
 
-include("services/copyFunctions.jl")
-
 # In Memory Types
 include("GraphsDFG/GraphsDFG.jl")
 include("LightDFG/LightDFG.jl")

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/services/LightDFG.jl

@@ -265,76 +265,31 @@ function getNeighbors(dfg::LightDFG, label::Symbol; solvable::Int=0)::Vector{Sym
 
 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")
+# TODO copy LightDFG to LightDFG overwrite
+# function copyGraph!(destDFG::LightDFG,
+#                     sourceDFG::LightDFG,
+#                     variableFactorLabels::Vector{Symbol};
+#                     copyGraphMetadata::Bool=false,
+#                     overwriteDest::Bool=false,
+#                     deepcopyNodes::Bool=false,
+#                     verbose::Bool = true)
 
-    #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]
+function buildSubgraph(::Type{G}, dfg::LightDFG, variableFactorLabels::Vector{Symbol}, distance::Int=0; solvable::Int=0, kwargs...) where G <: AbstractDFG
 
-    for v in values(sourceDFG.g.variables)
-        if v.label in labels
-            exists(destDFG, v) ? (@warn "_copyIntoGraph $(v.label) exists, ignoring pending error behaviour decision") : addVariable!(destDFG, deepcopy(v))
-        end
-    end
+    # find neighbors at distance to add
+    nbhood = Int[]
 
-    # And then all factors to the destDFG.
-    for f in values(sourceDFG.g.factors)
-        if f.label in labels
-            # Get the original factor variables (we need them to create it)
-            neigh_ints = FactorGraphs.outneighbors(sourceDFG.g, sourceDFG.g.labels[f.label])
-
-            neigh_labels = [sourceDFG.g.labels[i] for i in neigh_ints]
-            # Find the labels and associated variables in our new subgraph
-            factVariables = V[]
-            for v_lab in neigh_labels
-                if haskey(destDFG.g.variables, v_lab)
-                    push!(factVariables, getVariable(destDFG, v_lab))
-                    #otherwise ignore
-                end
-            end
-
-            # Only if we have all of them should we add it (otherwise strange things may happen on evaluation)
-            if includeOrphanFactors || length(factVariables) == length(neigh_labels)
-                exists(destDFG, f.label) ? (@warn "_copyIntoGraph $(f.label) exists, ignoring pending error behaviour decision") : addFactor!(destDFG, deepcopy(f))
-            end
-        end
-    end
-    return nothing
-end
-
-
-function getSubgraphAroundNode(dfg::LightDFG{P,V,F}, node::DFGNode, distance::Int64=1, includeOrphanFactors::Bool=false, addToDFG::LightDFG=LightDFG{P,V,F}(); solvable::Int=0)::LightDFG where {P <: AbstractParams, V <: AbstractDFGVariable, F <: AbstractDFGFactor}
-    if !exists(dfg,node.label)
-        error("Variable/factor with label '$(node.label)' does not exist in the factor graph")
+    for l in variableFactorLabels
+        union!(nbhood, neighborhood(dfg.g, dfg.g.labels[l], distance))
     end
 
-    # Get a list of all unique neighbors inside 'distance'
-    ns = neighborhood(dfg.g, dfg.g.labels[node.label], distance)
-    # Always return the center node, skip that if we're filtering.
-    solvable != 0 && (filter!(id -> _isSolvable(dfg, dfg.g.labels[id], solvable) || dfg.g.labels[id] == node.label, ns))
-
+    solvable != 0 && (filter!(id -> _isSolvable(dfg, dfg.g.labels[id], solvable), nbhood))
 
+    allvarfacs = [dfg.g.labels[id] for id in nbhood]
     # Copy the section of graph we want
-    _copyIntoGraph!(dfg, addToDFG, ns, includeOrphanFactors)
-    return addToDFG
-
-end
-# dfg::LightDFG{P,V,F}
-# where {P <: AbstractParams, V <: AbstractDFGVariable, F <: AbstractDFGFactor}
-
-function getSubgraph(dfg::LightDFG{P,V,F}, variableFactorLabels::Vector{Symbol}, includeOrphanFactors::Bool=false, addToDFG::LightDFG=LightDFG{P,V,F}())::LightDFG where {P <: AbstractParams, V <: AbstractDFGVariable, F <: AbstractDFGFactor}
-    for label in variableFactorLabels
-        if !exists(dfg, label)
-            error("Variable/factor with label '$(label)' does not exist in the factor graph")
-        end
-    end
-
-    variableFactorInts = [dfg.g.labels[s] for s in variableFactorLabels]
-
-    _copyIntoGraph!(dfg, addToDFG, variableFactorInts, includeOrphanFactors)
-    return addToDFG
+    destDFG = deepcopyGraph(G, dfg, allvarfacs; kwargs...)
+    return destDFG
 end