Skip to content

Commit c871442

Browse files
committed
Changes from review and finish changing build -> getSubgraph (probably still wrong verbNoun)
1 parent ea15ebc commit c871442

6 files changed

Lines changed: 40 additions & 32 deletions

File tree

src/Deprecated.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ end
436436
# - Orphaned factors (where the subgraph does not contain all the related variables) are not included.
437437
# Related:
438438
# - [`copyGraph!`](@ref)
439-
# - [`buildSubgraph`](@ref)
439+
# - [`getSubgraph`](@ref)
440440
# - [`listNeighborhood`](@ref)
441441
# - [`deepcopyGraph`](@ref)
442442
# """
@@ -455,7 +455,7 @@ NOTE: `copyGraphMetadata` is deprecated – use agent/graph Bloblets instead.
455455
Related:
456456
- [`deepcopyGraph`](@ref)
457457
- [`deepcopyGraph!`](@ref)
458-
- [`buildSubgraph`](@ref)
458+
- [`getSubgraph`](@ref)
459459
- [`listNeighborhood`](@ref)
460460
- [`mergeGraph!`](@ref)
461461
"""
@@ -528,7 +528,7 @@ Copy nodes from one graph into another graph by making deepcopies.
528528
see [`copyGraph!`](@ref) for more detail.
529529
Related:
530530
- [`deepcopyGraph`](@ref)
531-
- [`buildSubgraph`](@ref)
531+
- [`getSubgraph`](@ref)
532532
- [`listNeighborhood`](@ref)
533533
- [`mergeGraph!`](@ref)
534534
"""
@@ -556,7 +556,7 @@ Copy nodes from one graph into a new graph by making deepcopies.
556556
see [`copyGraph!`](@ref) for more detail.
557557
Related:
558558
- [`deepcopyGraph!`](@ref)
559-
- [`buildSubgraph`](@ref)
559+
- [`getSubgraph`](@ref)
560560
- [`listNeighborhood`](@ref)
561561
- [`mergeGraph!`](@ref)
562562
"""

src/GraphsDFG/GraphsDFG.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import ...DistributedFactorGraphs:
5656
listNeighbors,
5757
getPaths,
5858
getPath,
59-
buildSubgraph,
59+
getSubgraph,
6060
getBiadjacencyMatrix,
6161
toDot,
6262
toDotFile,

src/GraphsDFG/services/GraphsDFG.jl

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -404,11 +404,11 @@ function getPaths(
404404
edgepath = Graphs.a_star(dfg.g, dfg.g.labels[from], dfg.g.labels[to], distmx, heuristic)
405405

406406
if isempty(edgepath)
407-
return @NamedTuple{path::Vector{Symbol}, dist::Int64}[]
407+
return @NamedTuple{path::Vector{Symbol}, dist::T}[]
408408
end
409409

410410
path = [dfg.g.labels[edgepath[1].src]]
411-
dist = 0
411+
dist = zero(T)
412412
for (; dst, src) in edgepath
413413
push!(path, dfg.g.labels[dst])
414414
dist += distmx[src, dst]
@@ -429,18 +429,26 @@ function getPaths(
429429
kwargs...,
430430
)
431431
# If the user provided restricted lists, build the subgraph automatically
432-
active_dfg = if variableLabels === nothing && factorLabels === nothing
433-
dfg
434-
else
435-
vlabels = something(variableLabels, listVariables(dfg))
436-
flabels = something(factorLabels, listFactors(dfg))
437-
labels = vcat(vlabels, flabels)
438-
DFG.getSubgraph(
439-
GraphsDFG{NoSolverParams, VariableSkeleton, FactorSkeleton},
440-
dfg,
441-
labels,
442-
)
443-
end
432+
active_dfg =
433+
if isa(dfg, GraphsDFG) && isnothing(variableLabels) && isnothing(factorLabels)
434+
dfg
435+
else
436+
vlabels = something(variableLabels, listVariables(dfg))
437+
flabels = something(factorLabels, listFactors(dfg))
438+
labels = vcat(vlabels, flabels)
439+
DFG.getSubgraph(
440+
GraphsDFG{NoSolverParams, VariableSkeleton, FactorSkeleton},
441+
dfg,
442+
labels,
443+
)
444+
end
445+
!hasVariable(active_dfg, from) &&
446+
!hasFactor(active_dfg, from) &&
447+
throw(DFG.LabelNotFoundError(from))
448+
!hasVariable(active_dfg, to) &&
449+
!hasFactor(active_dfg, to) &&
450+
throw(DFG.LabelNotFoundError(to))
451+
444452
return getPaths(algorithm, active_dfg, from, to, k; kwargs...)
445453
end
446454

src/services/AbstractDFG.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ function getSubgraph(
565565
if !isnothing(solvable)
566566
Base.depwarn(
567567
"solvable kwarg is deprecated, use kwarg `solvableFilter = (>=solvable)` instead", #v0.29
568-
:buildSubgraph,
568+
:getSubgraph,
569569
)
570570
!isnothing(solvableFilter) &&
571571
error("Cannot use both solvable and solvableFilter kwargs.")

test/iifCompareTests.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ end
107107
addVariable!(fg, :l1, ContinuousScalar)
108108
addFactor!(fg, [:x1; :l1], LinearRelative(Rayleigh()))
109109

110-
sfg = buildSubgraph(GraphsDFG, fg, [:x0; :x1])
110+
sfg = getSubgraph(GraphsDFG, fg, [:x0; :x1])
111111

112112
@warn "FIXME This is NOT supposed to pass"
113113
@test_skip compareFactorGraphs(fg, sfg, skip = [:labelDict; :addHistory; :logpath])

test/testBlocks.jl

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,47 +1518,47 @@ function BuildingSubgraphs(testDFGAPI; VARTYPE = VariableDFG, FACTYPE = FactorDF
15181518
# "Getting Subgraphs"
15191519
dfg, verts, facs = connectivityTestGraph(testDFGAPI, VARTYPE, FACTYPE)
15201520
# Subgraphs
1521-
dfgSubgraph = buildSubgraph(testDFGAPI, dfg, [verts[1].label], 2)
1521+
dfgSubgraph = getSubgraph(testDFGAPI, dfg, [verts[1].label], 2)
15221522
# Only returns x1 and x2
15231523
@test issetequal([:x1, :x1x2f1, :x2], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15241524
#
1525-
dfgSubgraph = buildSubgraph(testDFGAPI, dfg, [:x1, :x2, :x1x2f1])
1525+
dfgSubgraph = getSubgraph(testDFGAPI, dfg, [:x1, :x2, :x1x2f1])
15261526
# Only returns x1 and x2
15271527
@test issetequal([:x1, :x1x2f1, :x2], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15281528

1529-
dfgSubgraph = buildSubgraph(testDFGAPI, dfg, [:x1x2f1], 1)
1529+
dfgSubgraph = getSubgraph(testDFGAPI, dfg, [:x1x2f1], 1)
15301530
# Only returns x1 and x2
15311531
@test issetequal([:x1, :x1x2f1, :x2], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15321532

15331533
#TODO if not a GraphsDFG with and summary or skeleton
15341534
if VARTYPE == VariableDFG
1535-
dfgSubgraph = buildSubgraph(testDFGAPI, dfg, [:x8], 2; solvableFilter = >=(1))
1535+
dfgSubgraph = getSubgraph(testDFGAPI, dfg, [:x8], 2; solvableFilter = >=(1))
15361536
@test issetequal([:x7], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15371537
#end if not a GraphsDFG with and summary or skeleton
15381538
end
15391539
# DFG issue #95 - confirming that getSubgraphAroundNode retains order
15401540
# REF: https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/issues/95
15411541
for fId in listVariables(dfg)
15421542
# Get a subgraph of this and it's related factors+variables
1543-
dfgSubgraph = buildSubgraph(testDFGAPI, dfg, [fId], 2)
1543+
dfgSubgraph = getSubgraph(testDFGAPI, dfg, [fId], 2)
15441544
# For each factor check that the order the copied graph == original
15451545
for fact in getFactors(dfgSubgraph)
15461546
@test fact.variableorder == getFactor(dfg, fact.label).variableorder
15471547
end
15481548
end
15491549

1550-
#TODO buildSubgraph default constructors for skeleton and summary
1550+
#TODO getSubgraph default constructors for skeleton and summary
15511551
if VARTYPE == VariableDFG
1552-
dfgSubgraph = buildSubgraph(dfg, [:x1, :x2, :x1x2f1])
1552+
dfgSubgraph = getSubgraph(dfg, [:x1, :x2, :x1x2f1])
15531553
@test issetequal([:x1, :x1x2f1, :x2], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15541554

1555-
dfgSubgraph = buildSubgraph(dfg, [:x2, :x3], 2)
1555+
dfgSubgraph = getSubgraph(dfg, [:x2, :x3], 2)
15561556
@test issetequal(
15571557
[:x2, :x3, :x1, :x4, :x3x4f1, :x1x2f1, :x2x3f1],
15581558
[ls(dfgSubgraph)..., lsf(dfgSubgraph)...],
15591559
)
15601560

1561-
dfgSubgraph = buildSubgraph(dfg, [:x1x2f1], 1)
1561+
dfgSubgraph = getSubgraph(dfg, [:x1x2f1], 1)
15621562
@test issetequal([:x1, :x1x2f1, :x2], [ls(dfgSubgraph)..., lsf(dfgSubgraph)...])
15631563
end
15641564
end
@@ -1893,14 +1893,14 @@ function PathFindingTests(testDFGAPI)
18931893

18941894
# --- Restrict with factorLabels only (all variables kept) ---
18951895
# Only allow the first 4 factors, path x1→x5 should still work
1896-
facs_first4 = listFactors(dfg; labelFilter = contains(r"x[1-4]"))
1896+
facs_first4 = listFactors(dfg; labelFilter = contains(r"x[1-4](?!\d)"))
18971897
result_fac = getPath(dfg, :x1, :x5; factorLabels = facs_first4)
18981898
@test first(result_fac.path) == :x1
18991899
@test last(result_fac.path) == :x5
19001900

19011901
# --- Restrict with both variableLabels and factorLabels ---
19021902
vars_1to5 = listVariables(dfg; typeFilter = ==(TestVariableType1()))
1903-
facs_1to4 = listFactors(dfg; labelFilter = contains(r"x[1-4]"))
1903+
facs_1to4 = listFactors(dfg; labelFilter = contains(r"x[1-4](?!\d)"))
19041904
result_both =
19051905
getPath(dfg, :x1, :x5; variableLabels = vars_1to5, factorLabels = facs_1to4)
19061906
@test first(result_both.path) == :x1

0 commit comments

Comments
 (0)