From 4db06a8f8ab1dd1ed993c1f9f3a2667241da055b Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 26 Aug 2025 18:00:33 +0200 Subject: [PATCH 1/2] ref verb and filter improvements --- src/Common.jl | 12 ++++++++++++ src/DataBlobs/services/BlobEntry.jl | 2 +- src/GraphsDFG/services/GraphsDFG.jl | 15 ++++++++++----- src/entities/AbstractDFG.jl | 2 +- src/entities/DFGFactor.jl | 3 +++ src/entities/DFGVariable.jl | 9 ++++++--- src/services/AbstractDFG.jl | 2 ++ src/services/CommonAccessors.jl | 5 +++++ src/services/CompareUtils.jl | 28 ++++++++++++++-------------- src/services/DFGFactor.jl | 3 ++- src/services/DFGVariable.jl | 4 ++-- 11 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/Common.jl b/src/Common.jl index 754c93eb..bf2f341f 100644 --- a/src/Common.jl +++ b/src/Common.jl @@ -126,6 +126,18 @@ function filterDFG!(nodes, predicate::Function, by::Function = identity) return filter!(predicate ∘ by, nodes) end +# specialized for label::Symbol filtering +function filterDFG!(nodes, predicate::Function, by::typeof(getLabel)) + # TODO this is not as clean as it should be, revisit if any issues arise + # Standard predicates that needs to be converted to string to work with Symbols + # OR look for the type if predicate isa Base.Fix2 && (predicate.x isa AbstractString || predicate.x isa Regex) + if predicate isa Base.Fix2 && typeof(predicate.f) in [typeof(contains), typeof(startswith), typeof(endswith)] + return filter!(predicate ∘ string ∘ by, nodes) + else + return filter!(predicate ∘ by, nodes) + end +end + ##============================================================================== ## Validation of session, robot, and user labels. ##============================================================================== diff --git a/src/DataBlobs/services/BlobEntry.jl b/src/DataBlobs/services/BlobEntry.jl index c6935c19..70504480 100644 --- a/src/DataBlobs/services/BlobEntry.jl +++ b/src/DataBlobs/services/BlobEntry.jl @@ -218,7 +218,7 @@ function getBlobentries( blobIdFilter::Union{Nothing, Function} = nothing, ) entries = getBlobentries(v) - filterDFG!(entries, labelFilter, x -> string(x.label)) + filterDFG!(entries, labelFilter, getLabel) filterDFG!(entries, blobIdFilter, x -> string(x.blobId)) return entries end diff --git a/src/GraphsDFG/services/GraphsDFG.jl b/src/GraphsDFG/services/GraphsDFG.jl index fb1e20d5..7cce7003 100644 --- a/src/GraphsDFG/services/GraphsDFG.jl +++ b/src/GraphsDFG/services/GraphsDFG.jl @@ -91,6 +91,11 @@ function mergeVariable!(dfg::GraphsDFG, variable::AbstractGraphVariable) return 1 end +function mergeVariables!(dfg::GraphsDFG, variables) + cnts = map(mergeVariable!, variables) + return sum(cnts) +end + function mergeFactor!(dfg::GraphsDFG, factor::AbstractGraphFactor) if !haskey(dfg.g.factors, factor.label) addFactor!(dfg, factor) @@ -174,7 +179,7 @@ function getVariables( filterDFG!(variables, >=(solvable), getSolvable) end - filterDFG!(variables, labelFilter, (String ∘ getLabel)) + filterDFG!(variables, labelFilter, getLabel) filterDFG!(variables, solvableFilter, getSolvable) filterDFG!(variables, tagsFilter, getTags) filterDFG!(variables, typeFilter, getVariableType) @@ -236,7 +241,7 @@ function getFactors( "The regex filter argument is deprecated, use kwarg `labelFilter=contains(regex)` instead", #v0.28 :getFactors, ) - filterDFG!(factors, contains(regex), (String ∘ getLabel)) + filterDFG!(factors, contains(regex), getLabel) end if !isempty(tags) # NOTE that !isdisjoint is not supported by NvaDFG. @@ -255,7 +260,7 @@ function getFactors( filterDFG!(factors, >=(solvable), getSolvable) end - filterDFG!(factors, labelFilter, (String ∘ getLabel)) + filterDFG!(factors, labelFilter, getLabel) filterDFG!(factors, solvableFilter, getSolvable) filterDFG!(factors, tagsFilter, getTags) filterDFG!(factors, typeFilter, typeof ∘ getFactorType) @@ -557,7 +562,7 @@ end function getGraphBlobentries(fg::GraphsDFG; labelFilter::Union{Nothing, Function} = nothing) entries = collect(values(fg.graph.blobEntries)) - filterDFG!(entries, labelFilter, (String ∘ getLabel)) + filterDFG!(entries, labelFilter, getLabel) return entries end @@ -566,7 +571,7 @@ function listGraphBlobentries( labelFilter::Union{Nothing, Function} = nothing, ) labels = collect(keys(fg.graph.blobEntries)) - filterDFG!(labels, labelFilter, String) + filterDFG!(labels, labelFilter, string) return labels end diff --git a/src/entities/AbstractDFG.jl b/src/entities/AbstractDFG.jl index 0a5daa51..71b2a25e 100644 --- a/src/entities/AbstractDFG.jl +++ b/src/entities/AbstractDFG.jl @@ -1,7 +1,7 @@ # TODO consider enforcing the full structure. # This is not explicitly inforced, but surves as extra information of how the structure is put together. -# AbstractDFGNode are all nodes that make up a DFG, including Agent, Graph, Variable, Factor, Blobstore, etc. +# AbstractDFGNode are all nodes that make up a DFG, including Agent, Graph, Variable, Factor, Blobstore, Blobentry etc. # abstract type AbstractDFGNode end # any DFGNode shall have a label. # abstract type AbstractGraphNode end <: AbstractDFGNode diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 4bb25c3e..e9987912 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -223,6 +223,9 @@ Base.@kwdef struct FactorCompute{FT <: AbstractObservation, N} <: AbstractGraphF solvercache::Base.RefValue{<:FactorCache} #TODO easy of use vs. performance as container is abstract in any case. end +#FIXME rename smallData to metadata +refMetadata(node::FactorCompute) = node.smallData + ##------------------------------------------------------------------------------ ## Constructors diff --git a/src/entities/DFGVariable.jl b/src/entities/DFGVariable.jl index 4c0f3dec..f3e84eac 100644 --- a/src/entities/DFGVariable.jl +++ b/src/entities/DFGVariable.jl @@ -44,7 +44,7 @@ Base.@kwdef mutable struct State{T <: StateType, P, N} Flag used by junction (Bayes) tree construction algorithm to know whether this variable has yet been included in the tree construction. """ eliminated::Bool = false - BayesNetVertID::Symbol = :NOTHING # Union{Nothing, } + BayesNetVertID::Symbol = :NOTHING # Union{Nothing, } #TODO deprecate separator::Vector{Symbol} = Symbol[] """ False if initial numerical values are not yet available or stored values are not ready for further processing yet. @@ -118,7 +118,7 @@ Base.@kwdef mutable struct PackedState dimIDs::Vector{Int} dims::Int eliminated::Bool - BayesNetVertID::Symbol # Int + BayesNetVertID::Symbol # Int #TODO deprecate separator::Vector{Symbol} # Int variableType::String initialized::Bool @@ -163,7 +163,6 @@ Data container to store Parameteric Point Estimate (PPE) for mean and max. """ Base.@kwdef struct MeanMaxPPE <: AbstractPointParametricEst id::Union{UUID, Nothing} = nothing # If it's blank it doesn't exist in the DB. - # repeat key value internally (from a design request by Sam) solveKey::Symbol suggested::Vector{Float64} max::Vector{Float64} @@ -326,6 +325,10 @@ Base.@kwdef struct VariableCompute{T <: StateType, P, N} <: AbstractGraphVariabl solvable::Base.RefValue{Int} = Ref(1) end +refStates(v::VariableCompute) = v.solverDataDict +refMetadata(v::VariableCompute) = v.smallData +refBlobentries(v::VariableCompute) = v.dataDict + ##------------------------------------------------------------------------------ ## Constructors diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl index 7e6082a1..110a849b 100644 --- a/src/services/AbstractDFG.jl +++ b/src/services/AbstractDFG.jl @@ -1099,6 +1099,8 @@ Related """ function findShortestPathDijkstra end + +#TODO deprecate """ $SIGNATURES diff --git a/src/services/CommonAccessors.jl b/src/services/CommonAccessors.jl index a428ae6d..71e0295b 100644 --- a/src/services/CommonAccessors.jl +++ b/src/services/CommonAccessors.jl @@ -1,6 +1,11 @@ ##============================================================================== ## Common Accessors ##============================================================================== + +refTags(node) = node.tags +refMetadata(node) = node.metadata +refBlobentries(node) = node.blobEntries # FIXME rename blobEntries to blobentries to match noun + # Common get and set methods # NOTE this could be reduced with macros and function generation to even less code. diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index e2ee8768..729d7df8 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -196,37 +196,37 @@ end #Compare State function compare(a::State, b::State) - a.val != b.val && @debug("val is not equal") == nothing && return false - a.bw != b.bw && @debug("bw is not equal") == nothing && return false + a.val != b.val && @debug("val is not equal") === nothing && return false + a.bw != b.bw && @debug("bw is not equal") === nothing && return false a.BayesNetOutVertIDs != b.BayesNetOutVertIDs && - @debug("BayesNetOutVertIDs is not equal") == nothing && + @debug("BayesNetOutVertIDs is not equal") === nothing && return false - a.dimIDs != b.dimIDs && @debug("dimIDs is not equal") == nothing && return false - a.dims != b.dims && @debug("dims is not equal") == nothing && return false + a.dimIDs != b.dimIDs && @debug("dimIDs is not equal") === nothing && return false + a.dims != b.dims && @debug("dims is not equal") === nothing && return false a.eliminated != b.eliminated && - @debug("eliminated is not equal") == nothing && + @debug("eliminated is not equal") === nothing && return false a.BayesNetVertID != b.BayesNetVertID && - @debug("BayesNetVertID is not equal") == nothing && + @debug("BayesNetVertID is not equal") === nothing && return false a.separator != b.separator && - @debug("separator is not equal") == nothing && + @debug("separator is not equal") === nothing && return false a.initialized != b.initialized && - @debug("initialized is not equal") == nothing && + @debug("initialized is not equal") === nothing && return false !isapprox(a.infoPerCoord, b.infoPerCoord; atol = 1e-13) && - @debug("infoPerCoord is not equal") == nothing && + @debug("infoPerCoord is not equal") === nothing && return false - a.ismargin != b.ismargin && @debug("ismargin is not equal") == nothing && return false + a.ismargin != b.ismargin && @debug("ismargin is not equal") === nothing && return false a.dontmargin != b.dontmargin && - @debug("dontmargin is not equal") == nothing && + @debug("dontmargin is not equal") === nothing && return false a.solveInProgress != b.solveInProgress && - @debug("solveInProgress is not equal") == nothing && + @debug("solveInProgress is not equal") === nothing && return false getVariableType(a) != getVariableType(b) && - @debug("variableType is not equal") == nothing && + @debug("variableType is not equal") === nothing && return false return true end diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 81af2009..acb5c87d 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -132,7 +132,7 @@ See documentation in [Manifolds.jl on making your own](https://juliamanifolds.gi Example: ``` -DFG.@defObservationType Pose2Pose2 RelativeObservation SpecialEuclidean(2) +DFG.@defObservationType Pose2Pose2 RelativeObservation SpecialEuclideanGroup(2) ``` """ macro defObservationType(structname, factortype, manifold) @@ -167,6 +167,7 @@ macro defObservationType(structname, factortype, manifold) end getManifold(obs::AbstractObservation) = getManifold(typeof(obs)) +getManifold(f::AbstractGraphFactor) = getManifold(getObservation(f)) ##============================================================================== ## Factors diff --git a/src/services/DFGVariable.jl b/src/services/DFGVariable.jl index 384da1a9..f134cc7e 100644 --- a/src/services/DFGVariable.jl +++ b/src/services/DFGVariable.jl @@ -92,7 +92,7 @@ See the [Manifolds.jl documentation on creating your own manifolds](https://juli Example: ``` -DFG.@defVariable Pose2 SpecialEuclidean(2) ArrayPartition([0;0.0],[1 0; 0 1.0]) +DFG.@defVariable Pose2 SpecialEuclideanGroup(2) ArrayPartition([0;0.0],[1 0; 0 1.0]) ``` """ macro defStateType(structname, manifold, point_identity) @@ -135,7 +135,7 @@ See the [Manifolds.jl documentation on creating your own manifolds](https://juli Example: ``` -DFG.@defStateTypeN Pose{N} SpecialEuclidean(N) ArrayPartition(zeros(SVector{N, Float64}), SMatrix{N, N, Float64}(I)) +DFG.@defStateTypeN Pose{N} SpecialEuclideanGroup(N) ArrayPartition(zeros(SVector{N, Float64}), SMatrix{N, N, Float64}(I)) ``` """ macro defStateTypeN(structname, manifold, point_identity) From 7331bfdae1303bd7c21005be418fedc21ea01791 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche <6612981+Affie@users.noreply.github.com> Date: Wed, 27 Aug 2025 12:44:48 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/Common.jl | 3 ++- src/services/AbstractDFG.jl | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Common.jl b/src/Common.jl index bf2f341f..8bd1ade9 100644 --- a/src/Common.jl +++ b/src/Common.jl @@ -131,7 +131,8 @@ function filterDFG!(nodes, predicate::Function, by::typeof(getLabel)) # TODO this is not as clean as it should be, revisit if any issues arise # Standard predicates that needs to be converted to string to work with Symbols # OR look for the type if predicate isa Base.Fix2 && (predicate.x isa AbstractString || predicate.x isa Regex) - if predicate isa Base.Fix2 && typeof(predicate.f) in [typeof(contains), typeof(startswith), typeof(endswith)] + if predicate isa Base.Fix2 && + typeof(predicate.f) in [typeof(contains), typeof(startswith), typeof(endswith)] return filter!(predicate ∘ string ∘ by, nodes) else return filter!(predicate ∘ by, nodes) diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl index 110a849b..44630414 100644 --- a/src/services/AbstractDFG.jl +++ b/src/services/AbstractDFG.jl @@ -1099,7 +1099,6 @@ Related """ function findShortestPathDijkstra end - #TODO deprecate """ $SIGNATURES