Skip to content

Commit e73e71d

Browse files
GearsADGearsAD
authored
Merge pull request #248 from JuliaRobotics/main/4Q19/consolidateSubfg
Consolidate, drop buildSubFG_SPECIAL
2 parents 05b6c1e + 7a4b3c0 commit e73e71d

File tree

1 file changed

+71
-56
lines changed

1 file changed

+71
-56
lines changed

‎src/needsahome.jl

Lines changed: 71 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ Notes
1414
- Slighly messy internals, but gets the job done -- some room for performance improvement.
1515
- Defaults to GraphDFG, but likely to change to LightDFG in future.
1616
17+
DevNotes
18+
- TODO: still needs to be consolidated with `DFG._copyIntoGraph`
19+
1720
Related
1821
1922
getVariableIds, _copyIntoGraph!
@@ -45,6 +48,16 @@ function buildSubgraphFromLabels!(dfg::G,
4548
deleteVariable!(subfg, dv)
4649
end
4750

51+
# delete any factors not in the allowed list
52+
if allowedFactors != nothing
53+
delFcts = setdiff(lsf(subfg), allowedFactors)
54+
for dfct in delFcts
55+
deleteFactor!(subfg, dfct)
56+
end
57+
end
58+
59+
# orphaned variables are allowed, but not orphaned factors
60+
4861
return subfg
4962
end
5063

@@ -59,59 +72,61 @@ function buildSubgraphFromLabels(dfg::G,
5972
end
6073

6174

62-
"""
63-
$SIGNATURES
64-
65-
IIF clique specific version of building subgraphs. This is was an unfortunate rewrite of the existing `buildSubgraphFromLabels!` function above. Currently halfway consolidated. Tests required to ensure these two functions can be reduced to and will perform the same in both.
66-
67-
DevNotes
68-
- DF: Could we somehow better consolidate the functionality of this method into `buildSubgraphFromLabels!` above, which in turn should be consolidated as SamC suggests.
69-
- Since this function has happened more than once, it seems the name `buildSubgraphFromLabels!` might stick around, even if it just becomes a wrapper.
70-
71-
Related
72-
73-
buildSubgraphFromLabels!, _copyIntoGraph!, getVariableIds
74-
"""
75-
function buildSubgraphFromLabels!_SPECIAL(dfg::G,
76-
# frontals::Vector{Symbol},
77-
syms::Vector{Symbol};
78-
subfg::AbstractDFG=(G <: InMemoryDFGTypes ? G : GraphsDFG)(params=getSolverParams(dfg)),
79-
solvable::Int=0,
80-
allowedFactors::Union{Nothing, Vector{Symbol}}=nothing )::G where G <: AbstractDFG
81-
#
82-
# for sym in separators
83-
# (solvable <= getSolvable(dfg, sym)) && DFG.addVariable!(subfg, deepcopy(DFG.getVariable(dfg, sym)))
84-
# end
85-
86-
addfac = Symbol[]
87-
for sym in syms # frontals
88-
if solvable <= getSolvable(dfg, sym)
89-
addVariable!(subfg, deepcopy(getVariable(dfg, sym)))
90-
append!(addfac, getNeighbors(dfg, sym, solvable=solvable))
91-
end
92-
end
93-
94-
# allowable factors as intersect between connected an user list
95-
usefac = allowedFactors == nothing ? addfac : intersect(allowedFactors, addfac)
96-
97-
allvars = ls(subfg)
98-
for sym in usefac
99-
fac = getFactor(dfg, sym)
100-
vos = fac._variableOrderSymbols
101-
#TODO don't add duplicates to start with
102-
if !exists(subfg,fac) && (vos ⊆ allvars) && (solvable <= getSolvable(dfg, sym))
103-
addFactor!(subfg, fac._variableOrderSymbols, deepcopy(fac))
104-
end
105-
end
106-
107-
# remove orphans
108-
for fct in getFactors(subfg)
109-
# delete any neighboring factors first
110-
if length(getNeighbors(subfg, fct)) != length(fct._variableOrderSymbols)
111-
deleteFactor!(subfg, fc)
112-
@error "deleteFactor! this should not happen"
113-
end
114-
end
115-
116-
return subfg
117-
end
75+
## KEEPING COMMENT, WANT TO BE CONSOLIDATED WITH FUNCTION ABOVE -- KEEPING ONLY ONE FOR MAINTAINABILITY
76+
## STILL NEEDS TO BE CONSOLIDATED WITH `DFG._copyIntoGraph`
77+
# """
78+
# $SIGNATURES
79+
#
80+
# IIF clique specific version of building subgraphs. This is was an unfortunate rewrite of the existing `buildSubgraphFromLabels!` function above. Currently halfway consolidated. Tests required to ensure these two functions can be reduced to and will perform the same in both.
81+
#
82+
# DevNotes
83+
# - DF: Could we somehow better consolidate the functionality of this method into `buildSubgraphFromLabels!` above, which in turn should be consolidated as SamC suggests.
84+
# - Since this function has happened more than once, it seems the name `buildSubgraphFromLabels!` might stick around, even if it just becomes a wrapper.
85+
#
86+
# Related
87+
#
88+
# buildSubgraphFromLabels!, _copyIntoGraph!, getVariableIds
89+
# """
90+
# function buildSubgraphFromLabels!_SPECIAL(dfg::G,
91+
# # frontals::Vector{Symbol},
92+
# syms::Vector{Symbol};
93+
# subfg::AbstractDFG=(G <: InMemoryDFGTypes ? G : GraphsDFG)(params=getSolverParams(dfg)),
94+
# solvable::Int=0,
95+
# allowedFactors::Union{Nothing, Vector{Symbol}}=nothing )::G where G <: AbstractDFG
96+
# #
97+
# # for sym in separators
98+
# # (solvable <= getSolvable(dfg, sym)) && DFG.addVariable!(subfg, deepcopy(DFG.getVariable(dfg, sym)))
99+
# # end
100+
#
101+
# addfac = Symbol[]
102+
# for sym in syms # frontals
103+
# if solvable <= getSolvable(dfg, sym)
104+
# addVariable!(subfg, deepcopy(getVariable(dfg, sym)))
105+
# append!(addfac, getNeighbors(dfg, sym, solvable=solvable))
106+
# end
107+
# end
108+
#
109+
# # allowable factors as intersect between connected an user list
110+
# usefac = allowedFactors == nothing ? addfac : intersect(allowedFactors, addfac)
111+
#
112+
# allvars = ls(subfg)
113+
# for sym in usefac
114+
# fac = getFactor(dfg, sym)
115+
# vos = fac._variableOrderSymbols
116+
# #TODO don't add duplicates to start with
117+
# if !exists(subfg,fac) && (vos ⊆ allvars) && (solvable <= getSolvable(dfg, sym))
118+
# addFactor!(subfg, fac._variableOrderSymbols, deepcopy(fac))
119+
# end
120+
# end
121+
#
122+
# # remove orphans
123+
# for fct in getFactors(subfg)
124+
# # delete any neighboring factors first
125+
# if length(getNeighbors(subfg, fct)) != length(fct._variableOrderSymbols)
126+
# deleteFactor!(subfg, fc)
127+
# @error "deleteFactor! this should not happen"
128+
# end
129+
# end
130+
#
131+
# return subfg
132+
# end

0 commit comments

Comments
 (0)