Enhance deleteVariables! deleteFactors! and plotDFG shows tags on hover (#1253)

* Enhance deleteVariables!|Factors!

* plotDFG shows tags on hover

* Fix Id errors and OrderedDict depr

* Test coverage and formatting

---------

Co-authored-by: Johannes Terblanche <Affie@users.noreply.github.com>

Author: Affie

ext/DFGPlots.jl

@@ -61,13 +61,20 @@ function plotDFG(dfg::GraphsDFG; p::DFGPlotProps = DFGPlotProps(), interactive::
         0;
         text = "",
         font = :bold,
-        fontsize = 30,
+        fontsize = 20,
         glowcolor = (:white, 1),
         glowwidth = 3,
     )
 
-    ax.aspect = GraphMakie.DataAspect()
+    ax.aspect = nothing
+    ax.autolimitaspect = 1
     if interactive
+        node_hover_labels = map(dfg.g.labels) do label
+            tags = sort(DFG.listTags(dfg, label))
+            tags_str = isempty(tags) ? "[]" : "[" * join(string.(tags), ", ") * "]"
+            return string(label, "\ntags: ", tags_str)
+        end
+
         function node_drag_action(state, idx, event, axis)
             p[:node_pos][][idx] = event.data
             return p[:node_pos][] = p[:node_pos][]
@@ -79,8 +86,7 @@ function plotDFG(dfg::GraphsDFG; p::DFGPlotProps = DFGPlotProps(), interactive::
         GraphMakie.register_interaction!(ax, :ndrag, ndrag)
 
         function node_hover_action(state, idx, event, axis)
-            label = dfg.g.labels[idx]
-            label_text.text[] = state ? string(label) : ""
+            label_text.text[] = state ? node_hover_labels[idx] : ""
             return label_text.transformation.translation[] = (event.data..., 0)
         end
         nhover = NodeHoverHandler(node_hover_action)

src/FileDFG/services/FileDFG.jl

@@ -117,6 +117,7 @@ function loadDFG!(
 
     # extract the factor graph from fileDFG folder
     variablefiles = readdir(joinpath(loaddir, "variables"); sort = false, join = true)
+    sort!(variablefiles; lt = natural_lt)
 
     # type instability on `variables` as either `::Vector{Variable}` or `::Vector{VariableDFG{<:}}` (vector of abstract)
     variables = @showprogress dt = 1 desc = "loading variables" asyncmap(
@@ -129,6 +130,7 @@ function loadDFG!(
     @debug "Loaded $(length(variables)) variables"
 
     factorfiles = readdir(joinpath(loaddir, "factors"); sort = false, join = true)
+    sort!(factorfiles; lt = natural_lt)
 
     factors = @showprogress dt = 1 desc = "loading factors" asyncmap(factorfiles) do file
         f = JSON.parsefile(file, F; style = DFGJSONStyle())
@@ -203,14 +205,14 @@ function loadDFG(file::AbstractString)
     blobproviders = if isfile(joinpath(loaddir, "blobproviders.json"))
         JSON.parsefile(
             joinpath(loaddir, "blobproviders.json"),
-            Dict{Symbol, AbstractBlobprovider};
+            OrderedDict{Symbol, AbstractBlobprovider};
             style = DFGJSONStyle(),
         )
     elseif isfile(joinpath(loaddir, "blobstores.json"))
         # backward compat: load old blobstores.json format
         JSON.parsefile(
             joinpath(loaddir, "blobstores.json"),
-            Dict{Symbol, AbstractBlobprovider};
+            OrderedDict{Symbol, AbstractBlobprovider};
             style = DFGJSONStyle(),
         )
     else

src/GraphsDFG/FactorGraphs/FactorGraphs.jl

@@ -18,6 +18,7 @@ import Graphs:
     add_vertex!,
     add_edge!,
     rem_vertex!,
+    rem_vertices!,
     rem_edge!,
     has_vertex,
     has_edge,
@@ -171,4 +172,27 @@ function rem_vertex!(g::FactorGraph{T, V, F}, v::Integer) where {T, V, F}
     return true
 end
 
+function Graphs.rem_vertices!(g::FactorGraph{T, V, F}, vs::Vector{Int}) where {T, V, F}
+    for v in sort(vs; rev = true)
+        v in vertices(g) || continue
+        lastv = nv(g)
+
+        rem_vertex!(g.graph, v) || continue
+
+        label = g.labels[v]
+        delete!(g.variables, label)
+        delete!(g.factors, label)
+
+        if v != lastv
+            g.labels[v] = g.labels[lastv] #lastSym
+        else
+            delete!(g.labels, v)
+        end
+    end
+
+    OrderedCollections.rehash!(g.variables)
+    OrderedCollections.rehash!(g.factors)
+    return true
+end
+
 end

src/GraphsDFG/services/factor_ops.jl

@@ -74,10 +74,9 @@ function DFG.deleteFactor!(dfg::GraphsDFG, label::Symbol)
 end
 
 function DFG.deleteFactors!(dfg::AbstractDFG, labels::Vector{Symbol})
-    counts = asyncmap(labels) do l
-        return deleteFactor!(dfg, l)
-    end
-    return sum(counts)
+    count = sum(map(l->hasFactor(dfg, l), labels); init = 0)
+    rem_vertices!(dfg.g, map(l->dfg.g.labels[l], labels))
+    return count
 end
 
 function DFG.deleteFactors!(dfg::AbstractDFG; kwargs...)

src/GraphsDFG/services/variable_ops.jl

@@ -77,12 +77,25 @@ function DFG.deleteVariable!(dfg::GraphsDFG, label::Symbol)#::Tuple{AbstractGrap
     !haskey(dfg.g.variables, label) && return 0
 
     # orphaned factors are not supported.
-    del_facs = map(l -> deleteFactor!(dfg, l), listNeighbors(dfg, label))
+    del_facs = deleteFactors!(dfg, listNeighbors(dfg, label))
 
     rem_vertex!(dfg.g, dfg.g.labels[label])
     return sum(del_facs; init = 0) + 1
 end
 
+function DFG.deleteVariables!(dfg::GraphsDFG, labels::Vector{Symbol})
+    # collect factor neighbors for all variables before any deletion
+    fac_labels = mapreduce(l -> listNeighbors(dfg, l), union, labels; init = Symbol[])
+    fac_labels = filter(l -> hasFactor(dfg, l), fac_labels)
+
+    var_labels = filter(l -> hasVariable(dfg, l), labels)
+
+    count = length(var_labels) + length(fac_labels)
+    vs = map(l -> dfg.g.labels[l], [var_labels; fac_labels])
+    rem_vertices!(dfg.g, vs)
+    return count
+end
+
 function DFG.listVariables(
     dfg::GraphsDFG;
     whereSolvable::Union{Nothing, Function} = nothing,

src/entities/Error.jl

@@ -44,41 +44,41 @@ function Base.showerror(io::IO, ex::LabelExistsError)
 end
 
 """
-    IdNotFoundError(Id, available)
+    IdNotFoundError(id, available)
 
-Error thrown when a requested Id is not found.
+Error thrown when a requested id is not found.
 """
-struct IdNotFoundError <: Exception
+struct IdNotFoundError{T} <: Exception
     name::String
-    Id::UUID
-    available::Vector{UUID}
+    id::T
+    available::Vector{T}
 end
 
-IdNotFoundError(name::String, Id::UUID) = IdNotFoundError(name, Id, UUID[])
-IdNotFoundError(Id::UUID) = IdNotFoundError("Node", Id, UUID[])
+IdNotFoundError(name::String, id::T) where {T} = IdNotFoundError(name, id, T[])
+IdNotFoundError(id::T) where {T} = IdNotFoundError("Node", id, T[])
 
 function Base.showerror(io::IO, ex::IdNotFoundError)
-    print(io, "IdNotFoundError: ", ex.name, " Id '", ex.Id, "' not found.")
+    print(io, "IdNotFoundError: ", ex.name, " id '", ex.id, "' not found.")
     if !isempty(ex.available)
-        println(io, " Available Ids:")
+        println(io, " Available ids:")
         show(io, ex.available)
     end
 end
 
 """
-    IdExistsError(Id)
+    IdExistsError(id)
 
-Error thrown when attempting to add an Id that already exists in the collection.
+Error thrown when attempting to add an id that already exists in the collection.
 """
-struct IdExistsError <: Exception
+struct IdExistsError{T} <: Exception
     name::String
-    Id::UUID
+    id::T
 end
 
-IdExistsError(Id::UUID) = IdExistsError("Node", Id)
+IdExistsError(id) = IdExistsError("Node", id)
 
 function Base.showerror(io::IO, ex::IdExistsError)
-    return print(io, "IdExistsError: ", ex.name, " Id '", ex.Id, "' already exists.")
+    return print(io, "IdExistsError: ", ex.name, " id '", ex.id, "' already exists.")
 end
 
 """

src/services/variable_ops.jl

@@ -121,7 +121,7 @@ function deleteVariables!(dfg::AbstractDFG, labels::Vector{Symbol})
     counts = asyncmap(labels) do l
         return deleteVariable!(dfg, l)
     end
-    return sum(counts)
+    return sum(counts; init = 0)
 end
 
 function deleteVariables!(dfg::AbstractDFG; kwargs...)

test/testBlocks.jl

@@ -460,15 +460,20 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2)
     @test addVariable!(fg, v3) === v3
     @test addFactor!(fg, f2) === f2
 
-    @test deleteFactor!(fg, f2) == 1
+    @test deleteFactors!(fg, [getLabel(f2)]) == 1
+    @test addFactor!(fg, f2) === f2
+    @test deleteFactors!(fg; whereLabel = ==(string(getLabel(f2)))) == 1
+    @test deleteFactors!(fg; whereLabel = ==("doesnotexist")) == 0
+
+    @test addFactor!(fg, f2) === f2
+    @test deleteVariables!(fg, [getLabel(v3)]) == 2
+    @test addVariable!(fg, v3) === v3
+    @test deleteVariables!(fg; whereLabel = ==(string(getLabel(v3)))) == 1
+    @test deleteVariables!(fg; whereLabel = ==("doesnotexist")) == 0
+
     @test deleteFactor!(fg, f2) == 0
     @test lsf(fg) == [:abf1]
 
-    delvarCompare = getVariable(fg, :c)
-    delfacCompare = []
-    ndel = deleteVariable!(fg, v3)
-    @test ndel == 1
-
     @test getVariable(fg, :a) == v1
 
     @test addFactor!(fg, f0) == f0