From 830f5077ff048451ffb6b49686770596a04176c5 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Wed, 2 Apr 2025 10:45:59 +0200 Subject: [PATCH 01/14] Spike on refactoring Factors --- Project.toml | 2 + src/DistributedFactorGraphs.jl | 1 + src/entities/DFGFactor.jl | 240 +++++++++++++++++++++------------ src/services/CustomPrinting.jl | 18 +-- src/services/DFGFactor.jl | 12 ++ src/services/Serialization.jl | 60 ++++++++- 6 files changed, 233 insertions(+), 100 deletions(-) diff --git a/Project.toml b/Project.toml index 6586b9af..19b12273 100644 --- a/Project.toml +++ b/Project.toml @@ -19,6 +19,7 @@ ManifoldsBase = "3362f125-f0bb-47a3-aa74-596ffd7ef2fb" OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d" Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f" ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca" +Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" RecursiveArrayTools = "731186ca-8d62-57ce-b412-fbd966d074cd" Reexport = "189a3867-3050-52da-a836-e630ba90ab69" SHA = "ea8e919c-243c-51af-8825-aaa63cd721ce" @@ -61,6 +62,7 @@ ManifoldsBase = "0.14, 0.15, 1" OrderedCollections = "1.4" Pkg = "1.4, 1.5" ProgressMeter = "1" +Random = "1.10" RecursiveArrayTools = "2, 3" Reexport = "1" SHA = "0.7, 1" diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 92a40252..3194d322 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -18,6 +18,7 @@ using Base using Base64 using DocStringExtensions using Dates +using Random using TimeZones using Distributions using Reexport diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 8b4b66f9..e3d9c011 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -49,6 +49,18 @@ Base.@kwdef mutable struct GenericFunctionNodeData{ inflation::Float64 = 0.0 end +#TODO is this mutable +@kwdef mutable struct FactorState + eliminated::Bool = false + potentialused::Bool = false + multihypo::Vector{Float64} = Float64[] # TODO re-evaluate after refactoring w #477 + certainhypo::Vector{Int} = Int[] + nullhypo::Float64 = 0.0 + solveInProgress::Int = 0 #TODO maybe deprecated or move to operational memory, also why Int? + inflation::Float64 = 0.0 +end + + # TODO should we move non FactorOperationalMemory to FactorCompute: # fnc, multihypo, nullhypo, inflation ? # that way we split solverData <: FactorOperationalMemory and constants @@ -81,10 +93,10 @@ FunctionNodeData(args...; kw...) = FunctionNodeData{typeof(args[4])}(args...; kw # # | | label | tags | timestamp | solvable | solverData | # |-------------------|:-----:|:----:|:---------:|:--------:|:----------:| -# | FactorSkeleton | X | x | | | | -# | FactorSummary | X | X | X | | | -# | FactorDFG | X | X | X | X | X* | -# | FactorCompute | X | X | X | X | X | +# | FactorSkeleton | X | x | | | | +# | FactorSummary | X | X | X | | | +# | FactorDFG | X | X | X | X | X* | +# | FactorCompute | X | X | X | X | X | # *not available without reconstruction """ @@ -92,10 +104,13 @@ FunctionNodeData(args...; kw...) = FunctionNodeData{typeof(args[4])}(args...; kw The Factor information packed in a way that accomdates multi-lang using json. """ + +#TODO do we have parameter for packed observation or is it already a string? +#TODO Same with metadata? Base.@kwdef struct FactorDFG <: AbstractDFGFactor id::Union{UUID, Nothing} = nothing label::Symbol - tags::Vector{Symbol} + tags::Set{Symbol} _variableOrderSymbols::Vector{Symbol} timestamp::ZonedDateTime nstime::String @@ -104,35 +119,93 @@ Base.@kwdef struct FactorDFG <: AbstractDFGFactor data::String metadata::String _version::String = string(_getDFGVersion()) + state::FactorState + observJSON::String # serialized opbservation # blobEntries::Vector{Blobentry}#TODO should factor have blob entries? end + #TODO type not in DFG FactorDFG, should it be? # _type::String # createdTimestamp::DateTime # lastUpdatedTimestamp::DateTime +StructTypes.StructType(::Type{FactorDFG}) = StructTypes.UnorderedStruct() +StructTypes.idproperty(::Type{FactorDFG}) = :id +StructTypes.omitempties(::Type{FactorDFG}) = (:id,) + +#TODO deprecate, added in v0.26 as a bridge to new serialization structure +function FactorDFG( + id::Union{UUID, Nothing}, + label::Symbol, + tags::Set{Symbol}, + _variableOrderSymbols::Vector{Symbol}, + timestamp::ZonedDateTime, + nstime::String, + fnctype::String, + solvable::Int, + data::String, + metadata::String, + _version::String, + state::Union{Nothing, FactorState}, + observJSON::Union{Nothing, String}, +) + if isnothing(state) || isnothing(observJSON) + fd = JSON3.read(data) + state = FactorState( + fd.eliminated, + fd.potentialused, + fd.multihypo, + fd.certainhypo, + fd.nullhypo, + fd.solveInProgress, + fd.inflation, + ) + observJSON = JSON3.write(fd.fnc) + end + return FactorDFG( + id, + label, + tags, + _variableOrderSymbols, + timestamp, + nstime, + fnctype, + solvable, + data, + metadata, + _version, + state, + observJSON, + ) +end + + FactorDFG(f::FactorDFG) = f # TODO consolidate to just one type """ $(TYPEDEF) Abstract parent type for all InferenceTypes, which are the -functions inside of factors. +observation functions inside of factors. """ -abstract type InferenceType <: DFG.AbstractPackedFactor end +abstract type InferenceType <: AbstractPackedFactor end + +#TODO deprecate InferenceType in favor of AbstractPackedFactor v0.26 + # this is the GenericFunctionNodeData for packed types -const FactorData = PackedFunctionNodeData{InferenceType} +#TODO deprecate FactorData in favor of FactorState (with no more distinction between packed and compute) +const FactorData = PackedFunctionNodeData{AbstractPackedFactor} # Packed Factor constructor function assembleFactorName(xisyms::Union{Vector{String}, Vector{Symbol}}) - return Symbol(xisyms..., "f_", string(uuid4())[1:4]) + return Symbol(xisyms..., "_f", randstring(4)) end -getFncTypeName(fnc::InferenceType) = split(string(typeof(fnc)), ".")[end] +getFncTypeName(fnc::AbstractPackedFactor) = split(string(typeof(fnc)), ".")[end] function FactorDFG( xisyms::Vector{Symbol}, - fnc::InferenceType; + fnc::AbstractPackedFactor; multihypo::Vector{Float64} = Float64[], nullhypo::Float64 = 0.0, solvable::Int = 1, @@ -144,7 +217,7 @@ function FactorDFG( metadata::Dict{Symbol, DFG.SmallDataTypes} = Dict{Symbol, DFG.SmallDataTypes}(), ) # create factor data - factordata = FactorData(; fnc, multihypo, nullhypo, inflation) + state = FactorState(; multihypo, nullhypo, inflation) fnctype = getFncTypeName(fnc) @@ -152,23 +225,21 @@ function FactorDFG( # create factor factor = FactorDFG(; label, - tags, + tags = Set(tags), _variableOrderSymbols = xisyms, timestamp, nstime = string(nstime), fnctype, solvable, - data = JSON3.write(factordata), metadata = base64encode(JSON3.write(metadata)), + state, + observJSON = JSON3.write(fnc), + data = "", #TODO deprecate data completely ) return factor end -StructTypes.StructType(::Type{FactorDFG}) = StructTypes.UnorderedStruct() -StructTypes.idproperty(::Type{FactorDFG}) = :id -StructTypes.omitempties(::Type{FactorDFG}) = (:id,) - ## FactorCompute lv2 """ @@ -183,9 +254,9 @@ DevNotes Fields: $(TYPEDFIELDS) """ -Base.@kwdef struct FactorCompute{T, N} <: AbstractDFGFactor +Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor """The ID for the factor""" - id::Union{UUID, Nothing} + id::Union{UUID, Nothing} = nothing """Factor label, e.g. :x1f1. Accessor: [`getLabel`](@ref)""" label::Symbol @@ -198,117 +269,112 @@ Base.@kwdef struct FactorCompute{T, N} <: AbstractDFGFactor """Variable timestamp. Accessors: [`getTimestamp`](@ref), [`setTimestamp`](@ref)""" timestamp::ZonedDateTime - """Nano second time, for more resolution on timestamp (only subsecond information)""" + """Nano second time""" nstime::Nanosecond """Solver data. Accessors: [`getSolverData`](@ref), [`setSolverData!`](@ref)""" - solverData::Base.RefValue{GenericFunctionNodeData{T}} + solverData::Base.RefValue{<:GenericFunctionNodeData} """Solvable flag for the factor. Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)""" solvable::Base.RefValue{Int} """Dictionary of small data associated with this variable. Accessors: [`getMetadata`](@ref), [`setMetadata!`](@ref)""" - smallData::Dict{Symbol, SmallDataTypes} - # Inner constructor - function FactorCompute{T}( - label::Symbol, - timestamp::Union{DateTime, ZonedDateTime}, - nstime::Nanosecond, - tags::Set{Symbol}, - solverData::GenericFunctionNodeData{T}, - solvable::Int, - _variableOrderSymbols::NTuple{N, Symbol}; - id::Union{UUID, Nothing} = nothing, - smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), - ) where {T, N} - return new{T, N}( - id, - label, - tags, - _variableOrderSymbols, - timestamp, - nstime, - Ref(solverData), - Ref(solvable), - smallData, - ) - end + smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}() + + #refactor fields + observation::FT + state::FactorState + computeMem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. + end ##------------------------------------------------------------------------------ ## Constructors -""" -$(SIGNATURES) +#TODO consolidate constructors, currently IIF calls only +# DFGFactor( +# Symbol(namestring), +# varOrderLabels, +# solverData; +# tags = Set(union(tags, [:FACTOR])), +# solvable, +# timestamp = _zonedtime(timestamp), +# ) -Construct a DFG factor given a label. -""" function FactorCompute( label::Symbol, timestamp::Union{DateTime, ZonedDateTime}, nstime::Nanosecond, tags::Set{Symbol}, - solverData::GenericFunctionNodeData{T}, + solverData::GenericFunctionNodeData, solvable::Int, - _variableOrderSymbols::Tuple; + variableOrder::Union{Vector{Symbol}, Tuple}; + observation = getFactorType(solverData), + state::FactorState = FactorState(), + computeMem::Base.RefValue{<:FactorOperationalMemory} = Ref{FactorOperationalMemory}(), id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), -) where {T} - return FactorCompute{T}( - label, - timestamp, - nstime, - tags, - solverData, - solvable, - _variableOrderSymbols; - id = id, - smallData = smallData, - ) -end - -function FactorCompute{T}( - label::Symbol, - variableOrderSymbols::Vector{Symbol}, - timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), - data::GenericFunctionNodeData{T} = GenericFunctionNodeData(; fnc = T()); - kw..., -) where {T} +) return FactorCompute( + id, label, + tags, + Tuple(variableOrder), timestamp, - Nanosecond(0), - Set{Symbol}(), - data, - 1, - Tuple(variableOrderSymbols); - kw..., + nstime, + Ref(solverData), + Ref(solvable), + smallData, + observation, + state, + computeMem, ) end -# # TODO standardize new fields in kw constructors, .id function FactorCompute( label::Symbol, - variableOrderSymbols::Vector{Symbol}, - data::GenericFunctionNodeData{T}; + variableOrder::Union{Vector{Symbol}, Tuple}, + solverData::GenericFunctionNodeData; tags::Set{Symbol} = Set{Symbol}(), timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), solvable::Int = 1, nstime::Nanosecond = Nanosecond(0), id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), -) where {T} - return FactorCompute{T}( +) + + observation = getFactorType(solverData) + + state = FactorState( + solverData.eliminated, + solverData.potentialused, + solverData.multihypo, + solverData.certainhypo, + solverData.nullhypo, + solverData.solveInProgress, + solverData.inflation, + ) + + if solverData.fnc isa FactorOperationalMemory + computeMem = Ref(solverData.fnc) + else + computeMem = Ref{FactorOperationalMemory}() + end + + return FactorCompute( label, timestamp, nstime, tags, - data, + solverData, solvable, - Tuple(variableOrderSymbols); + Tuple(variableOrder); + observation, + computeMem, id, smallData, + state, ) end diff --git a/src/services/CustomPrinting.jl b/src/services/CustomPrinting.jl index 793d17dd..c8d1b0a7 100644 --- a/src/services/CustomPrinting.jl +++ b/src/services/CustomPrinting.jl @@ -119,13 +119,10 @@ function printFactor( ioc = IOContext(io, :limit => limit, :compact => compact) if short - opmemt = (getSolverData(vert).fnc |> typeof).name.name - fct = getFactorType(vert) + fct = getObservation(vert) fctt = fct |> typeof - printstyled(ioc, typeof(vert).name.name, "{", opmemt, "{"; bold = true) - printstyled(ioc, fctt.name.name; bold = true, color = :blue) - printstyled(ioc, "...}}"; bold = true) - println(ioc) + printstyled(ioc, summary(vert); bold = true) + println() println(ioc, " ID: ", vert.id) println(ioc, " timestamp: ", vert.timestamp) println(ioc, " nstime: ", vert.nstime) @@ -134,16 +131,15 @@ function printFactor( println(ioc) println(ioc, " solvable: ", vert.solvable) println(ioc, " VariableOrder: ", vert._variableOrderSymbols) - println(ioc, " multihypo: ", getSolverData(vert).multihypo) # FIXME #477 - println(ioc, " nullhypo: ", getSolverData(vert).nullhypo) + println(ioc, " multihypo: ", getState(vert).multihypo) # FIXME #477 + println(ioc, " nullhypo: ", getState(vert).nullhypo) println(ioc, " tags: ", vert.tags) printstyled(ioc, " FactorType: "; bold = true, color = :blue) println(ioc, fctt) # show(ioc, fctt) for f in setdiff(fieldnames(fctt), skipfields) - printstyled(ioc, f, ":"; color = :magenta) - println(ioc) - show(ioc, typeof(getproperty(fct, f)).name.name) + printstyled(ioc, f, ": "; color = :magenta) + show(ioc, typeof(getproperty(fct, f))) println(ioc) end else diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 9629720e..2588175a 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -2,6 +2,10 @@ ## Accessors ##============================================================================== +function getMetadata(f::FactorDFG) + return JSON3.read(base64decode(f.metadata), Dict{Symbol, SmallDataTypes}) +end + ##============================================================================== ## GenericFunctionNodeData ##============================================================================== @@ -33,6 +37,14 @@ getFactorType(fct::FactorCompute) = getFactorType(getSolverData(fct)) getFactorType(f::FactorDFG) = getTypeFromSerializationModule(f.fnctype)() # TODO find a better way to do this that does not rely on empty constructor getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl)) +getState(f::AbstractDFGFactor) = f.state +getObservation(f::FactorCompute) = f.observation +function getObservation(f::FactorDFG) + #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes + packtype = DFG.getTypeFromSerializationModule("Packed" * f.fnctype) + return packtype(; JSON3.read(f.observJSON)...) +end + """ $SIGNATURES diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 808bd567..9257e51f 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -273,7 +273,7 @@ function packFactor(f::FactorCompute) return FactorDFG(; id = f.id, label = f.label, - tags = collect(f.tags), + tags = f.tags, _variableOrderSymbols = f._variableOrderSymbols, timestamp = f.timestamp, nstime = string(f.nstime.value), @@ -283,6 +283,8 @@ function packFactor(f::FactorCompute) # Pack the node data data = JSON3.write(_packSolverData(f, fnctype)), _version = string(_getDFGVersion()), + state = f.state, + observJSON = JSON3.write(packObservation(f)), ) return props end @@ -378,10 +380,12 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo @debug "DECODING factor type = '$(factor.fnctype)' for factor '$(factor.label)'" !skipVersionCheck && _versionCheck(factor) - fullFactorData = nothing + local fullFactorData + local observation try if factor.data[1] == '{' packedFnc = fncStringToData(factor.fnctype, factor.data) + observation = unpackObservation(factor) else packedFnc = fncStringToData(factor.fnctype, String(base64decode(factor.data))) end @@ -398,6 +402,12 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo metadata = JSON3.read(base64decode(factor.metadata), Dict{Symbol, DFG.SmallDataTypes}) + if fullFactorData.fnc isa FactorOperationalMemory + computeMem = Ref(fullFactorData.fnc) + else + computeMem = Ref{FactorOperationalMemory}() + end + return FactorCompute( factor.label, factor.timestamp, @@ -408,6 +418,52 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo Tuple(factor._variableOrderSymbols); id = factor.id, smallData = metadata, + state = factor.state, + observation, + computeMem, + ) +end + +function unpackObservation(factor::FactorDFG) + #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes + observpacked = getObservation(factor) + #TODO change to unpack: observ = unpack(observpacked) + packtype = DFG.getTypeFromSerializationModule("Packed" * factor.fnctype) + return convert(convertStructType(packtype), observpacked) +end + +packObservation(f::FactorCompute) = packObservation(getObservation(f)) +function packObservation(observ::AbstractFactor) + packtype = convertPackedType(observ) + return convert(packtype, observ) +end + +function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) + # + @debug "DECODING factor type = '$(factor.fnctype)' for factor '$(factor.label)'" + !skipVersionCheck && _versionCheck(factor) + + local observation + try + observation = unpackObservation(factor) + catch + @error "Error while unpacking '$(factor.label)' as '$(factor.fnctype)', please check the unpacking/packing converters for this factor" + rethrow() + end + + return FactorCompute( + factor.id, + factor.label, + factor.tags, + Tuple(factor._variableOrderSymbols), + factor.timestamp, + Nanosecond(factor.nstime), + Ref{GenericFunctionNodeData}(), + Ref(factor.solvable), + getMetadata(factor), + observation, + factor.state, + Ref{FactorOperationalMemory}(), ) end From 14edc098dc40fe639cfe014a48b03821c7b38d60 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche <6612981+Affie@users.noreply.github.com> Date: Wed, 2 Apr 2025 10:50:01 +0200 Subject: [PATCH 02/14] Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/entities/DFGFactor.jl | 5 ----- src/services/Serialization.jl | 3 +-- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index e3d9c011..b058d231 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -60,7 +60,6 @@ end inflation::Float64 = 0.0 end - # TODO should we move non FactorOperationalMemory to FactorCompute: # fnc, multihypo, nullhypo, inflation ? # that way we split solverData <: FactorOperationalMemory and constants @@ -179,7 +178,6 @@ function FactorDFG( ) end - FactorDFG(f::FactorDFG) = f # TODO consolidate to just one type @@ -285,7 +283,6 @@ Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor observation::FT state::FactorState computeMem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. - end ##------------------------------------------------------------------------------ @@ -343,9 +340,7 @@ function FactorCompute( id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), ) - observation = getFactorType(solverData) - state = FactorState( solverData.eliminated, solverData.potentialused, diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 9257e51f..bfa09243 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -407,7 +407,6 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo else computeMem = Ref{FactorOperationalMemory}() end - return FactorCompute( factor.label, factor.timestamp, @@ -426,7 +425,7 @@ end function unpackObservation(factor::FactorDFG) #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes - observpacked = getObservation(factor) + observpacked = getObservation(factor) #TODO change to unpack: observ = unpack(observpacked) packtype = DFG.getTypeFromSerializationModule("Packed" * factor.fnctype) return convert(convertStructType(packtype), observpacked) From 3d6c96dd11c8699ece5494eb882482b9046de38a Mon Sep 17 00:00:00 2001 From: Johannes Terblanche <6612981+Affie@users.noreply.github.com> Date: Thu, 12 Jun 2025 06:40:46 -0400 Subject: [PATCH 03/14] defFactorType macro (#1077) * defFactorFunction macro * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * update defFactorType * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update DFGFactor.jl --------- Co-authored-by: Johannes Terblanche Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- src/entities/DFGFactor.jl | 1 - src/services/DFGFactor.jl | 53 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index b058d231..6787ee97 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -2,7 +2,6 @@ ## Abstract Types ##============================================================================== -# TODO consider changing this to AbstractFactor abstract type AbstractFactor end abstract type AbstractPackedFactor end diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 2588175a..86361fac 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -64,6 +64,59 @@ function _getPriorType(_type::Type{<:InferenceVariable}) return getfield(_type.name.module, Symbol(:Prior, _type.name.name)) end +##============================================================================== +## Default Factors Function Macro +##============================================================================== +export PackedSamplableBelief +# export pack, unpack, packDistribution, unpackDistribution + +function pack end +function unpack end +function packDistribution end +function unpackDistribution end + +abstract type PackedSamplableBelief end +StructTypes.StructType(::Type{<:PackedSamplableBelief}) = StructTypes.UnorderedStruct() + +""" + @defFactorType StructName factortype<:AbstractFactor manifolds<:ManifoldsBase.AbstractManifold + +A macro to create a new factor function with name `StructName` and manifolds. Note that +the `manifolds` is an object and *must* be a subtype of `ManifoldsBase.AbstractManifold`. +See documentation in [Manifolds.jl on making your own](https://juliamanifolds.github.io/Manifolds.jl/stable/examples/manifold.html). + +Example: +``` +DFG.@defFactorType Pose2Pos2 AbstractManifoldMinimize SpecialEuclidean(2) +``` +""" +macro defFactorType(structname, factortype, manifold) + packedstructname = Symbol("Packed", structname) + return esc( + quote + Base.@__doc__ struct $structname{T} <: $factortype + Z::T + end + + Base.@__doc__ struct $packedstructname{T <: PackedSamplableBelief} <: + AbstractPackedFactor + Z::T + end + + # user manifold must be a <:Manifold + @assert ($manifold isa AbstractManifold) "@defFactorType of " * + string($structname) * + " requires that the " * + string($manifold) * + " be a subtype of `ManifoldsBase.AbstractManifold`" + + DFG.getManifold(::Type{$structname}) = $manifold + DFG.pack(d::$structname) = $packedstructname(DFG.packDistribution(d.Z)) + DFG.unpack(d::$packedstructname) = $structname(DFG.unpackDistribution(d.Z)) + end, + ) +end + ##============================================================================== ## Factors ##============================================================================== From c45ba31d58692d852d9cc3f093dc2f0fe9c39b7d Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Thu, 12 Jun 2025 19:22:22 +0200 Subject: [PATCH 04/14] Fix tests --- src/services/CompareUtils.jl | 15 +++++++++++++-- src/services/DFGFactor.jl | 3 ++- test/plottingTest.jl | 3 ++- test/testBlocks.jl | 10 +++++----- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index a3f3d4f9..99047266 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -30,10 +30,11 @@ const GeneratedCompareUnion = Union{ FactorDFG, FactorSummary, FactorSkeleton, + FactorState, } @generated function ==(x::T, y::T) where {T <: GeneratedCompareUnion} - ignored = [] + ignored = [:computeMem] return mapreduce( n -> :(x.$n == y.$n), (a, b) -> :($a && $b), @@ -315,7 +316,17 @@ function compareFactor( skipcompute::Bool = true, ) # - skip_ = union([:attributes; :solverData; :_variableOrderSymbols; :_gradients], skip) + skip_ = union( + [ + :attributes, + :solverData, + :observation, + :computeMem, + :_variableOrderSymbols, + :_gradients, + ], + skip, + ) TP = compareAll(A, B; skip = skip_, show = show) @debug "compareFactor 1/5" TP TP = diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 86361fac..fd037140 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -32,7 +32,8 @@ Return user factor type from factor graph identified by label `::Symbol`. Notes - Replaces older `getfnctype`. """ -getFactorType(data::GenericFunctionNodeData) = data.fnc.usrfnc! +getFactorType(data::GenericFunctionNodeData{<:FactorOperationalMemory}) = data.fnc.usrfnc! +getFactorType(data::GenericFunctionNodeData{<:AbstractFactor}) = data.fnc getFactorType(fct::FactorCompute) = getFactorType(getSolverData(fct)) getFactorType(f::FactorDFG) = getTypeFromSerializationModule(f.fnctype)() # TODO find a better way to do this that does not rely on empty constructor getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl)) diff --git a/test/plottingTest.jl b/test/plottingTest.jl index 0099defa..2ec2159c 100644 --- a/test/plottingTest.jl +++ b/test/plottingTest.jl @@ -17,9 +17,10 @@ map(v -> addVariable!(dfg, v), verts) map( n -> addFactor!( dfg, - FactorCompute{TestFunctorInferenceType1}( + FactorCompute( Symbol("x$(n)x$(n+1)f1"), [verts[n].label, verts[n + 1].label], + GenericFunctionNodeData(; fnc=TestFunctorInferenceType1()) ), ), 1:(numNodes - 1), diff --git a/test/testBlocks.jl b/test/testBlocks.jl index 5c9af8c1..a27cb525 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -395,13 +395,13 @@ function DFGFactorSCA() gfnd = GenericFunctionNodeData(; fnc = TestCCW(TestFunctorInferenceType1())) - f1 = FactorCompute{TestCCW{TestFunctorInferenceType1}}(f1_lbl, [:a, :b]) f1 = FactorCompute(f1_lbl, [:a, :b], gfnd; tags = f1_tags, solvable = 0) - f2 = FactorCompute{TestCCW{TestFunctorInferenceType1}}( + f2 = FactorCompute( :bcf1, [:b, :c], - ZonedDateTime("2020-08-11T00:12:03.000-05:00"), + GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}()); + timestamp = ZonedDateTime("2020-08-11T00:12:03.000-05:00"), ) #TODO add tests for mutating vos in updateFactor and orphan related checks. # we should perhaps prevent an empty vos @@ -473,7 +473,7 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) @test getLabel(fg[getLabel(v1)]) == getLabel(v1) #TODO standardize this error and res also for that matter - fnope = FactorCompute{TestCCW{TestFunctorInferenceType1}}(:broken, [:a, :nope]) + fnope = FactorCompute(:broken, [:a, :nope], GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}())) @test_throws KeyError addFactor!(fg, fnope) @test addFactor!(fg, f1) == f1 @@ -1683,7 +1683,7 @@ function ProducingDotFiles( end if f1 === nothing if (FACTYPE == FactorCompute) - f1 = FactorCompute{TestFunctorInferenceType1}(:abf1, [:a, :b]) + f1 = FactorCompute(:abf1, [:a, :b], GenericFunctionNodeData(;fnc = TestFunctorInferenceType1())) else f1 = FACTYPE(:abf1, [:a, :b]) end From eba3b38f5beea685ac0ba602a5ae9b42b061ddc8 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Fri, 13 Jun 2025 16:23:48 +0200 Subject: [PATCH 05/14] towards consolidated FactorCompute constructor --- src/Deprecated.jl | 33 ++++++++++++++++ src/entities/DFGFactor.jl | 73 ++++++++++++++++++++--------------- src/services/CompareUtils.jl | 4 +- src/services/DFGFactor.jl | 19 ++++++--- src/services/Serialization.jl | 22 +++++------ test/testBlocks.jl | 10 ++--- 6 files changed, 106 insertions(+), 55 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index f6fdbbc6..f93966fb 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -190,6 +190,39 @@ function updateVariableSolverData!( end end +function FactorCompute( + label::Symbol, + timestamp::Union{DateTime, ZonedDateTime}, + nstime::Nanosecond, + tags::Set{Symbol}, + solverData::GenericFunctionNodeData, + solvable::Int, + variableOrder::Union{Vector{Symbol}, Tuple}; + observation = getFactorType(solverData), + state::FactorState = FactorState(), + workmem::Base.RefValue{<:FactorOperationalMemory} = Ref{FactorOperationalMemory}(), + id::Union{UUID, Nothing} = nothing, + smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), +) + error( + "This constructor is deprecated, use FactorCompute(label, variableOrder, solverData; ...) instead", + ) + return FactorCompute( + id, + label, + tags, + Tuple(variableOrder), + timestamp, + nstime, + Ref(solverData), + Ref(solvable), + smallData, + observation, + state, + workmem, + ) +end + ## ================================================================================ ## Deprecated in v0.25 ##================================================================================= diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 6787ee97..9750a826 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -253,7 +253,7 @@ $(TYPEDFIELDS) """ Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor """The ID for the factor""" - id::Union{UUID, Nothing} = nothing + id::Union{UUID, Nothing} = nothing #TODO deprecate id """Factor label, e.g. :x1f1. Accessor: [`getLabel`](@ref)""" label::Symbol @@ -279,38 +279,46 @@ Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}() #refactor fields + """Observation function or measurement for this factor. + Accessors: [`getObservation`](@ref)(@ref)""" observation::FT + """Describes the current state of the factor. Persisted in serialization. + Accessors: [`getFactorState`](@ref)""" state::FactorState - computeMem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. + """Temporary, non-persistent memory used internally by the solver for intermediate numerical computations and buffers. + `workmem` is lazily allocated and only used during factor operations; it is not serialized or retained after solving. + Accessors: [`getWorkmem`](@ref), [`setWorkmem!`](@ref)""" + workmem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. end ##------------------------------------------------------------------------------ ## Constructors -#TODO consolidate constructors, currently IIF calls only -# DFGFactor( -# Symbol(namestring), -# varOrderLabels, -# solverData; -# tags = Set(union(tags, [:FACTOR])), -# solvable, -# timestamp = _zonedtime(timestamp), -# ) - +# TODO standardize new fields in kw constructors, .id function FactorCompute( label::Symbol, - timestamp::Union{DateTime, ZonedDateTime}, - nstime::Nanosecond, - tags::Set{Symbol}, - solverData::GenericFunctionNodeData, - solvable::Int, - variableOrder::Union{Vector{Symbol}, Tuple}; - observation = getFactorType(solverData), + variableOrder::Union{Vector{Symbol}, Tuple}, + observation::AbstractFactor, state::FactorState = FactorState(), - computeMem::Base.RefValue{<:FactorOperationalMemory} = Ref{FactorOperationalMemory}(), + workmem = Ref{FactorOperationalMemory}(); + tags::Set{Symbol} = Set{Symbol}(), + timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), + solvable::Int = 1, + nstime::Nanosecond = Nanosecond(0), id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), + solverData = nothing ) + if isnothing(solverData) + refsolverData = Ref{GenericFunctionNodeData}() #TODO solverData deprecated in v0.27 WIP + else + Base.depwarn( + "`FactorCompute` solverData is deprecated", + :FactorCompute, + ) + refsolverData = Ref(solverData) + end + return FactorCompute( id, label, @@ -318,16 +326,15 @@ function FactorCompute( Tuple(variableOrder), timestamp, nstime, - Ref(solverData), + refsolverData, Ref(solvable), smallData, observation, state, - computeMem, + workmem, ) end -# TODO standardize new fields in kw constructors, .id function FactorCompute( label::Symbol, variableOrder::Union{Vector{Symbol}, Tuple}, @@ -339,6 +346,10 @@ function FactorCompute( id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), ) + Base.depwarn( + "`FactorCompute` constructor with `GenericFunctionNodeData` is deprecated. observation, state, and workmem should be provided explicitly.", + :FactorCompute, + ) observation = getFactorType(solverData) state = FactorState( solverData.eliminated, @@ -351,24 +362,24 @@ function FactorCompute( ) if solverData.fnc isa FactorOperationalMemory - computeMem = Ref(solverData.fnc) + workmem = Ref(solverData.fnc) else - computeMem = Ref{FactorOperationalMemory}() + workmem = Ref{FactorOperationalMemory}() end return FactorCompute( label, + Tuple(variableOrder), + observation, + state, + workmem; + id, timestamp, nstime, tags, solverData, - solvable, - Tuple(variableOrder); - observation, - computeMem, - id, smallData, - state, + solvable, ) end diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index 99047266..26a39c91 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -34,7 +34,7 @@ const GeneratedCompareUnion = Union{ } @generated function ==(x::T, y::T) where {T <: GeneratedCompareUnion} - ignored = [:computeMem] + ignored = [:workmem] return mapreduce( n -> :(x.$n == y.$n), (a, b) -> :($a && $b), @@ -321,7 +321,7 @@ function compareFactor( :attributes, :solverData, :observation, - :computeMem, + :workmem, :_variableOrderSymbols, :_gradients, ], diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index fd037140..69af1038 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -39,6 +39,10 @@ getFactorType(f::FactorDFG) = getTypeFromSerializationModule(f.fnctype)() # TODO getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl)) getState(f::AbstractDFGFactor) = f.state + +getFactorState(f::AbstractDFGFactor) = f.state +getFactorState(dfg::AbstractDFG, lbl::Symbol) = getFactorState(getFactor(dfg, lbl)) + getObservation(f::FactorCompute) = f.observation function getObservation(f::FactorDFG) #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes @@ -46,6 +50,9 @@ function getObservation(f::FactorDFG) return packtype(; JSON3.read(f.observJSON)...) end +getWorkmem(f::FactorCompute) = f.workmem[] +setWorkmem!(f::FactorCompute, workmem::FactorOperationalMemory) = f.workmem[] = workmem + """ $SIGNATURES @@ -155,12 +162,12 @@ end function setTimestamp(f::FactorCompute, ts::ZonedDateTime) return FactorCompute( f.label, - ts, - f.nstime, - f.tags, - f.solverData, - f.solvable, - getfield(f, :_variableOrderSymbols); + getfield(f, :_variableOrderSymbols), + f.solverData; + timestamp = ts, + nstime = f.nstime, + tags = f.tags, + solvable = f.solvable, id = f.id, ) end diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index bfa09243..6d86899c 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -403,23 +403,23 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo metadata = JSON3.read(base64decode(factor.metadata), Dict{Symbol, DFG.SmallDataTypes}) if fullFactorData.fnc isa FactorOperationalMemory - computeMem = Ref(fullFactorData.fnc) + workmem = Ref(fullFactorData.fnc) else - computeMem = Ref{FactorOperationalMemory}() + workmem = Ref{FactorOperationalMemory}() end return FactorCompute( factor.label, - factor.timestamp, - Nanosecond(factor.nstime), - Set(factor.tags), - fullFactorData, - factor.solvable, - Tuple(factor._variableOrderSymbols); + Tuple(factor._variableOrderSymbols), + observation, + factor.state, + workmem; + solverData = fullFactorData, + nstime = Nanosecond(factor.nstime), + tags = Set(factor.tags), + solvable = factor.solvable, + timestamp = factor.timestamp, id = factor.id, smallData = metadata, - state = factor.state, - observation, - computeMem, ) end diff --git a/test/testBlocks.jl b/test/testBlocks.jl index a27cb525..245b11e5 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -493,12 +493,12 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) if f2 isa FactorCompute f2_mod = FactorCompute( f2.label, - f2.timestamp, - f2.nstime, - f2.tags, - f2.solverData, - f2.solvable, (:a,), + f2.solverData; + timestamp = f2.timestamp, + nstime = f2.nstime, + tags = f2.tags, + solvable = f2.solvable, ) else f2_mod = deepcopy(f2) From 66666f14975d25699cc1fc36a7b8955081cdaaeb Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Sat, 14 Jun 2025 16:46:55 +0200 Subject: [PATCH 06/14] Intermediate removing factor solverData --- src/Deprecated.jl | 4 ++++ src/entities/DFGFactor.jl | 44 +++++++++++++++++++++++------------ src/services/CompareUtils.jl | 25 +++++++++++--------- src/services/DFGFactor.jl | 23 ++++++++++++++---- src/services/Serialization.jl | 33 +++++++++++++++----------- test/fileDFGTests.jl | 6 ++--- test/testBlocks.jl | 3 +-- 7 files changed, 89 insertions(+), 49 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index f93966fb..2adcf5b6 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -73,6 +73,10 @@ function updateVariableSolverData!( fields::Vector{Symbol} = Symbol[]; warn_if_absent::Bool = true, ) + Base.depwarn( + "updateVariableSolverData! is deprecated, use mergeVariableState! or copytoVariableState! instead", + :updateVariableSolverData!, + ) #This is basically just setSolverData var = getVariable(dfg, variablekey) warn_if_absent && diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 9750a826..d173a894 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -114,11 +114,11 @@ Base.@kwdef struct FactorDFG <: AbstractDFGFactor nstime::String fnctype::String solvable::Int - data::String + data::Union{String, Nothing} = nothing #TODO deprecated in v0.27 remove data completely, use state and observJSON metadata::String _version::String = string(_getDFGVersion()) state::FactorState - observJSON::String # serialized opbservation + observJSON::String # serialized observation # blobEntries::Vector{Blobentry}#TODO should factor have blob entries? end @@ -131,7 +131,7 @@ StructTypes.StructType(::Type{FactorDFG}) = StructTypes.UnorderedStruct() StructTypes.idproperty(::Type{FactorDFG}) = :id StructTypes.omitempties(::Type{FactorDFG}) = (:id,) -#TODO deprecate, added in v0.26 as a bridge to new serialization structure +#TODO deprecate, added in v0.27 as a bridge to new serialization structure function FactorDFG( id::Union{UUID, Nothing}, label::Symbol, @@ -141,7 +141,7 @@ function FactorDFG( nstime::String, fnctype::String, solvable::Int, - data::String, + data::Union{Nothing, String}, metadata::String, _version::String, state::Union{Nothing, FactorState}, @@ -300,25 +300,28 @@ function FactorCompute( variableOrder::Union{Vector{Symbol}, Tuple}, observation::AbstractFactor, state::FactorState = FactorState(), - workmem = Ref{FactorOperationalMemory}(); + _workmem = nothing; tags::Set{Symbol} = Set{Symbol}(), timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), solvable::Int = 1, nstime::Nanosecond = Nanosecond(0), id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), - solverData = nothing + solverData = nothing, ) if isnothing(solverData) refsolverData = Ref{GenericFunctionNodeData}() #TODO solverData deprecated in v0.27 WIP else - Base.depwarn( - "`FactorCompute` solverData is deprecated", - :FactorCompute, - ) + Base.depwarn("`FactorCompute` solverData is deprecated", :FactorCompute) refsolverData = Ref(solverData) end - + + if isnothing(_workmem) + workmem = Ref{FactorOperationalMemory}() + else + workmem = Ref(_workmem) + end + return FactorCompute( id, label, @@ -362,9 +365,9 @@ function FactorCompute( ) if solverData.fnc isa FactorOperationalMemory - workmem = Ref(solverData.fnc) + workmem = solverData.fnc else - workmem = Ref{FactorOperationalMemory}() + workmem = nothing end return FactorCompute( @@ -383,9 +386,20 @@ function FactorCompute( ) end -Base.getproperty(x::FactorCompute, f::Symbol) = begin - if f == :solvable || f == :solverData +function Base.getproperty(x::FactorCompute, f::Symbol) + if f == :solvable getfield(x, f)[] + elseif f == :solverData + Base.depwarn( + "`FactorCompute` field `$(f)` is deprecated, use `getObservation`, `getState` or `getWorkmem` instead.", + :getproperty, + ) + # error("WIP removing direct access, breaking change in v0.27, use `state` or `Workmem` instead.") + if isassigned(getfield(x, f)) + getfield(x, f)[] + else + nothing + end elseif f == :_variableOrderSymbols [getfield(x, f)...] else diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index 26a39c91..6e4e4b3b 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -34,7 +34,7 @@ const GeneratedCompareUnion = Union{ } @generated function ==(x::T, y::T) where {T <: GeneratedCompareUnion} - ignored = [:workmem] + ignored = [:workmem, :solverData] return mapreduce( n -> :(x.$n == y.$n), (a, b) -> :($a && $b), @@ -330,9 +330,9 @@ function compareFactor( TP = compareAll(A, B; skip = skip_, show = show) @debug "compareFactor 1/5" TP TP = - TP & compareAllSpecial( - getSolverData(A), - getSolverData(B); + TP & compareAll( + getState(A), + getState(B); skip = union([:fnc; :_gradients], skip), show = show, ) @@ -341,9 +341,9 @@ function compareFactor( return TP end TP = - TP & compareAllSpecial( - getSolverData(A).fnc, - getSolverData(B).fnc; + TP & compareAll( + getObservation(A), + getObservation(B); skip = union( [ :cpt @@ -359,7 +359,9 @@ function compareFactor( show = show, ) @debug "compareFactor 3/5" TP - if !(:measurement in skip) + + #FIXME is measurement stil in use and should it be checked, skipping for now + if false # !(:measurement in skip) TP = TP & ( skipsamples || compareAll( @@ -371,7 +373,8 @@ function compareFactor( ) end @debug "compareFactor 4/5" TP - if !(:varValsAll in skip) && hasfield(typeof(getSolverData(A).fnc), :varValsAll) + #FIXME is varValsAll stil in use and should it be checked, skipping for now + if false #!(:varValsAll in skip) && hasfield(typeof(getSolverData(A).fnc), :varValsAll) TP = TP & ( skipcompute || compareAll( @@ -383,8 +386,8 @@ function compareFactor( ) end @debug "compareFactor 5/5" TP - if !(:varidx in skip) && - hasfield(typeof(getSolverData(A).fnc), :varidx) && + #FIXME is varidx stil in use and should it be checked, skipping for now + if false #!(:varidx in skip) && hasfield(typeof(getSolverData(A).fnc), :varidx) && getSolverData(A).fnc.varidx isa Base.RefValue TP = TP & ( diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 69af1038..27d8eafc 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -21,7 +21,7 @@ end Return reference to the user factor in `<:AbstractDFG` identified by `::Symbol`. """ getFactorFunction(fcd::GenericFunctionNodeData) = fcd.fnc.usrfnc! -getFactorFunction(fc::FactorCompute) = getFactorFunction(getSolverData(fc)) +getFactorFunction(fc::FactorCompute) = getObservation(fc) getFactorFunction(dfg::AbstractDFG, fsym::Symbol) = getFactorFunction(getFactor(dfg, fsym)) """ @@ -32,9 +32,17 @@ Return user factor type from factor graph identified by label `::Symbol`. Notes - Replaces older `getfnctype`. """ -getFactorType(data::GenericFunctionNodeData{<:FactorOperationalMemory}) = data.fnc.usrfnc! -getFactorType(data::GenericFunctionNodeData{<:AbstractFactor}) = data.fnc -getFactorType(fct::FactorCompute) = getFactorType(getSolverData(fct)) +function getFactorType(data::GenericFunctionNodeData{<:FactorOperationalMemory}) + #TODO deprecated in v0.27 + Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) + return data.fnc.usrfnc! +end +function getFactorType(data::GenericFunctionNodeData{<:AbstractFactor}) + #TODO deprecated in v0.27 + Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) + return data.fnc +end +getFactorType(fct::FactorCompute) = getObservation(fct) getFactorType(f::FactorDFG) = getTypeFromSerializationModule(f.fnctype)() # TODO find a better way to do this that does not rely on empty constructor getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl)) @@ -225,7 +233,12 @@ getVariableOrder(dfg::AbstractDFG, fct::Symbol) = getVariableOrder(getFactor(dfg Retrieve solver data structure stored in a factor. """ function getSolverData(f::FactorCompute) - return f.solverData + Base.depwarn("getSolverData(f::FactorCompute) is deprecated", :getSolverData) + if isassigned(getfield(f, :solverData)) + return getfield(f, :solverData)[] + else + return nothing + end end setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) = f.solverData = data diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 6d86899c..473e28d3 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -253,9 +253,13 @@ VariableDFG(v::VariableCompute) = packVariable(v) function _packSolverData(f::FactorCompute, fnctype::AbstractFactor) # + Base.depwarn( + "_packSolverData is deprecated, use seperate packing of observation #TODO", + :_packSolverData, + ) packtype = convertPackedType(fnctype) try - packed = convert(PackedFunctionNodeData{packtype}, getSolverData(f)) + packed = convert(PackedFunctionNodeData{packtype}, getSolverData(f)) #TODO getSolverData packedJson = packed return packedJson catch ex @@ -269,7 +273,7 @@ end # returns FactorDFG function packFactor(f::FactorCompute) - fnctype = getSolverData(f).fnc.usrfnc! + fnctype = getObservation(f) return FactorDFG(; id = f.id, label = f.label, @@ -281,7 +285,6 @@ function packFactor(f::FactorCompute) solvable = getSolvable(f), metadata = base64encode(JSON3.write(f.smallData)), # Pack the node data - data = JSON3.write(_packSolverData(f, fnctype)), _version = string(_getDFGVersion()), state = f.state, observJSON = JSON3.write(packObservation(f)), @@ -383,15 +386,19 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo local fullFactorData local observation try - if factor.data[1] == '{' - packedFnc = fncStringToData(factor.fnctype, factor.data) - observation = unpackObservation(factor) + observation = unpackObservation(factor) + if !isnothing(factor.data) + if factor.data[1] == '{' + packedFnc = fncStringToData(factor.fnctype, factor.data) + else + packedFnc = fncStringToData(factor.fnctype, String(base64decode(factor.data))) + end + decodeType = getFactorOperationalMemoryType(dfg) + fullFactorData = + decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) else - packedFnc = fncStringToData(factor.fnctype, String(base64decode(factor.data))) + fullFactorData = nothing end - decodeType = getFactorOperationalMemoryType(dfg) - fullFactorData = - decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) catch ex io = IOBuffer() showerror(io, ex, catch_backtrace()) @@ -402,10 +409,10 @@ function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Boo metadata = JSON3.read(base64decode(factor.metadata), Dict{Symbol, DFG.SmallDataTypes}) - if fullFactorData.fnc isa FactorOperationalMemory - workmem = Ref(fullFactorData.fnc) + if !isnothing(fullFactorData) && fullFactorData.fnc isa FactorOperationalMemory + workmem = fullFactorData.fnc else - workmem = Ref{FactorOperationalMemory}() + workmem = nothing end return FactorCompute( factor.label, diff --git a/test/fileDFGTests.jl b/test/fileDFGTests.jl index 7f21d57a..b8ec87ff 100644 --- a/test/fileDFGTests.jl +++ b/test/fileDFGTests.jl @@ -80,8 +80,8 @@ using UUIDs 1:(numNodes - 1), ) map(f -> setSolvable!(f, Int(round(rand()))), facts) - map(f -> f.solverData.eliminated = rand() > 0.5, facts) - map(f -> f.solverData.potentialused = rand() > 0.5, facts) + map(f -> DFG.getState(f).eliminated = rand() > 0.5, facts) + map(f -> DFG.getState(f).potentialused = rand() > 0.5, facts) mergeFactor!.(dfg, facts) #test multihypo @@ -135,7 +135,7 @@ using UUIDs @test compareFactor( getFactor(dfg, fact), getFactor(retDFG, fact), - skip = [:timezone, :zone], + skip = [:timezone, :zone, :solverData,], ) # Timezones # :hypotheses, :certainhypo, :multihypo, # Multihypo # :eliminated, diff --git a/test/testBlocks.jl b/test/testBlocks.jl index 245b11e5..1197768f 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -794,8 +794,7 @@ function VSDTestBlock!(fg, v1) # Delete it @test deleteVariableSolverData!(fg, :a, :parametric) == 1 # Update add it - @test @test_logs (:warn, r"does not exist") updateVariableSolverData!(fg, :a, vnd) == - vnd + @test mergeVariableState!(fg, :a, vnd) == 1 # Update update it @test updateVariableSolverData!(fg, :a, vnd) == vnd From d0dbc75fca01ccf88bce703582d54b6c6092d886 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 17 Jun 2025 13:16:12 +0200 Subject: [PATCH 07/14] rm factor data --- src/Deprecated.jl | 43 ++++++++++++ src/DistributedFactorGraphs.jl | 4 +- src/FileDFG/services/FileDFG.jl | 22 ++++--- src/entities/DFGFactor.jl | 29 +++------ src/services/AbstractDFG.jl | 4 +- src/services/CompareUtils.jl | 4 +- src/services/DFGFactor.jl | 68 +++++++++---------- src/services/Serialization.jl | 112 ++++++++++---------------------- test/testBlocks.jl | 9 +-- 9 files changed, 141 insertions(+), 154 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 2adcf5b6..e96b83a4 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -227,6 +227,49 @@ function FactorCompute( ) end +function getSolverData(f::FactorCompute) + return error( + "getSolverData(f::FactorCompute) is obsolete, use getState, getObservation, or getWorkmem instead", + ) +end + +function setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) + return error( + "setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) is obsolete, use setState!, or setWorkmem! instead", + ) +end + +@deprecate unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Bool = false) unpackFactor( + factor; + skipVersionCheck, +) + +@deprecate rebuildFactorMetadata!(args...; kwargs...) rebuildFactorWorkmem!( + args...; + kwargs..., +) + +export reconstFactorData +function reconstFactorData end + +function decodePackedType( + dfg::AbstractDFG, + varOrder::AbstractVector{Symbol}, + ::Type{T}, + packeddata::GenericFunctionNodeData{PT}, +) where {T <: FactorOperationalMemory, PT} + error("decodePackedType is obsolete") + # + # TODO, to solve IIF 1424 + # variables = map(lb->getVariable(dfg, lb), varOrder) + + # Also look at parentmodule + usrtyp = convertStructType(PT) + fulltype = DFG.FunctionNodeData{T{usrtyp}} + factordata = reconstFactorData(dfg, varOrder, fulltype, packeddata) + return factordata +end + ## ================================================================================ ## Deprecated in v0.25 ##================================================================================= diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 3194d322..5557e3cf 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -262,7 +262,7 @@ export mergeVariableData!, mergeGraphVariableData! # Serialization type conversion export convertPackedType, convertStructType -export reconstFactorData +export pack, unpack, packDistribution, unpackDistribution ##------------------------------------------------------------------------------ ## Other utility functions @@ -286,7 +286,7 @@ export findClosestTimestamp, findVariableNearTimestamp # Serialization export packVariable, unpackVariable, packFactor, unpackFactor -export rebuildFactorMetadata! +export rebuildFactorWorkmem! export @defVariable # File import and export diff --git a/src/FileDFG/services/FileDFG.jl b/src/FileDFG/services/FileDFG.jl index 83a2cfa7..380c196c 100644 --- a/src/FileDFG/services/FileDFG.jl +++ b/src/FileDFG/services/FileDFG.jl @@ -99,7 +99,6 @@ function loadDFG!( dst::AbstractString; overwriteDFGMetadata::Bool = true, ) - # # loaddir gets deleted so needs to be unique loaddir = split(joinpath("/", "tmp", "caesar", "random", string(uuid1())), '-')[1] @@ -176,9 +175,10 @@ function loadDFG!( end @info "Loaded $(length(variables)) variables"#- $(map(v->v.label, variables))" - @info "Inserting variables into graph..." - # Adding variables - map(v -> addVariable!(dfgLoadInto, v), variables) + @showprogress 1 "Inserting variables into graph" map( + v -> addVariable!(dfgLoadInto, v), + variables, + ) usePackedFactor = isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) == FactorDFG @@ -190,21 +190,23 @@ function loadDFG!( if usePackedFactor return packedfact else - return unpackFactor(dfgLoadInto, packedfact) + return unpackFactor(packedfact) end end @info "Loaded $(length(factors)) factors"# - $(map(f->f.label, factors))" - @info "Inserting factors into graph..." # # Adding factors - map(f -> addFactor!(dfgLoadInto, f), factors) + @showprogress 1 "Inserting factors into graph" map( + f -> addFactor!(dfgLoadInto, f), + factors, + ) if isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) != FactorDFG # Finally, rebuild the CCW's for the factors to completely reinflate them # NOTE CREATES A NEW FactorCompute IF CCW TYPE CHANGES - @info "Rebuilding CCW's for the factors..." - @showprogress 1 "build factor operational memory" for factor in factors - rebuildFactorMetadata!(dfgLoadInto, factor) + # @info "Rebuilding CCW's for the factors..." + @showprogress 1 "Rebuilding factor working memory" for factor in factors + rebuildFactorWorkmem!(dfgLoadInto, factor) end end diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index d173a894..3918c07c 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -114,7 +114,6 @@ Base.@kwdef struct FactorDFG <: AbstractDFGFactor nstime::String fnctype::String solvable::Int - data::Union{String, Nothing} = nothing #TODO deprecated in v0.27 remove data completely, use state and observJSON metadata::String _version::String = string(_getDFGVersion()) state::FactorState @@ -169,7 +168,6 @@ function FactorDFG( nstime, fnctype, solvable, - data, metadata, _version, state, @@ -268,9 +266,6 @@ Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor timestamp::ZonedDateTime """Nano second time""" nstime::Nanosecond - """Solver data. - Accessors: [`getSolverData`](@ref), [`setSolverData!`](@ref)""" - solverData::Base.RefValue{<:GenericFunctionNodeData} """Solvable flag for the factor. Accessors: [`getSolvable`](@ref), [`setSolvable!`](@ref)""" solvable::Base.RefValue{Int} @@ -309,11 +304,8 @@ function FactorCompute( smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), solverData = nothing, ) - if isnothing(solverData) - refsolverData = Ref{GenericFunctionNodeData}() #TODO solverData deprecated in v0.27 WIP - else + if !isnothing(solverData) Base.depwarn("`FactorCompute` solverData is deprecated", :FactorCompute) - refsolverData = Ref(solverData) end if isnothing(_workmem) @@ -329,7 +321,6 @@ function FactorCompute( Tuple(variableOrder), timestamp, nstime, - refsolverData, Ref(solvable), smallData, observation, @@ -380,7 +371,6 @@ function FactorCompute( timestamp, nstime, tags, - solverData, smallData, solvable, ) @@ -390,16 +380,9 @@ function Base.getproperty(x::FactorCompute, f::Symbol) if f == :solvable getfield(x, f)[] elseif f == :solverData - Base.depwarn( - "`FactorCompute` field `$(f)` is deprecated, use `getObservation`, `getState` or `getWorkmem` instead.", - :getproperty, + error( + "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getState` or `getWorkmem` instead.", ) - # error("WIP removing direct access, breaking change in v0.27, use `state` or `Workmem` instead.") - if isassigned(getfield(x, f)) - getfield(x, f)[] - else - nothing - end elseif f == :_variableOrderSymbols [getfield(x, f)...] else @@ -408,8 +391,12 @@ function Base.getproperty(x::FactorCompute, f::Symbol) end function Base.setproperty!(x::FactorCompute, f::Symbol, val) - if f == :solvable || f == :solverData + if f == :solvable getfield(x, f)[] = val + elseif f == :solverData + error( + "`solverData` is obsolete in `FactorCompute`. Use `Observation`, `State` or `Workmem` instead.", + ) else setfield!(x, f, val) end diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl index 2b39a629..6d54727e 100644 --- a/src/services/AbstractDFG.jl +++ b/src/services/AbstractDFG.jl @@ -116,12 +116,12 @@ end Method must be overloaded by the user for Serialization to work. """ -function rebuildFactorMetadata!( +function rebuildFactorWorkmem!( dfg::AbstractDFG{<:AbstractParams}, factor::AbstractDFGFactor, neighbors = [], ) - return error("rebuildFactorMetadata! is not implemented for $(typeof(dfg))") + return error("rebuildFactorWorkmem! is not implemented for $(typeof(dfg))") end """ diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index 6e4e4b3b..f8c01048 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -359,7 +359,7 @@ function compareFactor( show = show, ) @debug "compareFactor 3/5" TP - + #FIXME is measurement stil in use and should it be checked, skipping for now if false # !(:measurement in skip) TP = @@ -388,7 +388,7 @@ function compareFactor( @debug "compareFactor 5/5" TP #FIXME is varidx stil in use and should it be checked, skipping for now if false #!(:varidx in skip) && hasfield(typeof(getSolverData(A).fnc), :varidx) && - getSolverData(A).fnc.varidx isa Base.RefValue + getSolverData(A).fnc.varidx isa Base.RefValue TP = TP & ( skipcompute || compareAll( diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 27d8eafc..fcff8348 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -56,9 +56,16 @@ function getObservation(f::FactorDFG) #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes packtype = DFG.getTypeFromSerializationModule("Packed" * f.fnctype) return packtype(; JSON3.read(f.observJSON)...) + # return packtype(JSON3.read(f.observJSON)) end -getWorkmem(f::FactorCompute) = f.workmem[] +function getWorkmem(f::FactorCompute) + if isassigned(f.workmem) + return f.workmem[] + else + return nothing + end +end setWorkmem!(f::FactorCompute, workmem::FactorOperationalMemory) = f.workmem[] = workmem """ @@ -84,7 +91,6 @@ end ## Default Factors Function Macro ##============================================================================== export PackedSamplableBelief -# export pack, unpack, packDistribution, unpackDistribution function pack end function unpack end @@ -94,38 +100,48 @@ function unpackDistribution end abstract type PackedSamplableBelief end StructTypes.StructType(::Type{<:PackedSamplableBelief}) = StructTypes.UnorderedStruct() +#TODO remove, rather use StructTypes.jl properly +function Base.convert(::Type{<:PackedSamplableBelief}, nt::Union{NamedTuple, JSON3.Object}) + distrType = getTypeFromSerializationModule(nt._type) + return distrType(; nt...) +end + """ - @defFactorType StructName factortype<:AbstractFactor manifolds<:ManifoldsBase.AbstractManifold + @defFactorType StructName factortype<:AbstractFactor manifolds<:AbstractManifold -A macro to create a new factor function with name `StructName` and manifolds. Note that -the `manifolds` is an object and *must* be a subtype of `ManifoldsBase.AbstractManifold`. +A macro to create a new factor function with name `StructName` and manifold. Note that +the `manifold` is an object and *must* be a subtype of `ManifoldsBase.AbstractManifold`. See documentation in [Manifolds.jl on making your own](https://juliamanifolds.github.io/Manifolds.jl/stable/examples/manifold.html). Example: ``` -DFG.@defFactorType Pose2Pos2 AbstractManifoldMinimize SpecialEuclidean(2) +DFG.@defFactorType Pose2Pose2 AbstractManifoldMinimize SpecialEuclidean(2) ``` """ macro defFactorType(structname, factortype, manifold) packedstructname = Symbol("Packed", structname) return esc( quote + # user manifold must be a <:Manifold + @assert ($manifold isa AbstractManifold) "@defFactorType manifold (" * + string($manifold) * + ") is not an `AbstractManifold`" + + @assert ($factortype <: AbstractFactor) "@defFactorType factortype (" * + string($factortype) * + ") is not an `AbstractFactor`" + Base.@__doc__ struct $structname{T} <: $factortype Z::T end - Base.@__doc__ struct $packedstructname{T <: PackedSamplableBelief} <: - AbstractPackedFactor - Z::T + #TODO should this be $packedstructname{T <: PackedSamplableBelief} + Base.@__doc__ struct $packedstructname <: AbstractPackedFactor + Z::PackedSamplableBelief end - # user manifold must be a <:Manifold - @assert ($manifold isa AbstractManifold) "@defFactorType of " * - string($structname) * - " requires that the " * - string($manifold) * - " be a subtype of `ManifoldsBase.AbstractManifold`" - + # $structname(; Z) = $structname(Z) + $packedstructname(; Z) = $packedstructname(Z) DFG.getManifold(::Type{$structname}) = $manifold DFG.pack(d::$structname) = $packedstructname(DFG.packDistribution(d.Z)) DFG.unpack(d::$packedstructname) = $structname(DFG.unpackDistribution(d.Z)) @@ -223,26 +239,6 @@ getVariableOrder(fct::FactorCompute) = fct._variableOrderSymbols::Vector{Symbol} getVariableOrder(fct::FactorDFG) = fct._variableOrderSymbols::Vector{Symbol} getVariableOrder(dfg::AbstractDFG, fct::Symbol) = getVariableOrder(getFactor(dfg, fct)) -##------------------------------------------------------------------------------ -## solverData -##------------------------------------------------------------------------------ - -""" - $SIGNATURES - -Retrieve solver data structure stored in a factor. -""" -function getSolverData(f::FactorCompute) - Base.depwarn("getSolverData(f::FactorCompute) is deprecated", :getSolverData) - if isassigned(getfield(f, :solverData)) - return getfield(f, :solverData)[] - else - return nothing - end -end - -setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) = f.solverData = data - ##------------------------------------------------------------------------------ ## utility ##------------------------------------------------------------------------------ diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 473e28d3..99297f3f 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -294,25 +294,6 @@ end packFactor(f::FactorDFG) = f -function reconstFactorData end - -function decodePackedType( - dfg::AbstractDFG, - varOrder::AbstractVector{Symbol}, - ::Type{T}, - packeddata::GenericFunctionNodeData{PT}, -) where {T <: FactorOperationalMemory, PT} - # - # TODO, to solve IIF 1424 - # variables = map(lb->getVariable(dfg, lb), varOrder) - - # Also look at parentmodule - usrtyp = convertStructType(PT) - fulltype = DFG.FunctionNodeData{T{usrtyp}} - factordata = reconstFactorData(dfg, varOrder, fulltype, packeddata) - return factordata -end - function fncStringToData(packtype::Type{<:AbstractPackedFactor}, data::String) # Read string as JSON object to use as kwargs @@ -378,72 +359,50 @@ function fncStringToData(fncType::String, data::Union{String, <:NamedTuple}) return fncStringToData(packtype, data) end -function unpackFactor(dfg::AbstractDFG, factor::FactorDFG; skipVersionCheck::Bool = false) - # - @debug "DECODING factor type = '$(factor.fnctype)' for factor '$(factor.label)'" - !skipVersionCheck && _versionCheck(factor) - - local fullFactorData - local observation +function unpackObservation(factor::FactorDFG) try - observation = unpackObservation(factor) - if !isnothing(factor.data) - if factor.data[1] == '{' - packedFnc = fncStringToData(factor.fnctype, factor.data) - else - packedFnc = fncStringToData(factor.fnctype, String(base64decode(factor.data))) - end - decodeType = getFactorOperationalMemoryType(dfg) - fullFactorData = - decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) + return unpack(getObservation(factor)) + catch e + if e isa MethodError && e.f == unpack + Base.depwarn( + """$e\nPlease implement pack and unpack methods for the factor type '$(typeof(getObservation(factor)))'. + Falling back to deprecated convert method.""", + :unpackObservation, + ) + #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes + #TODO change to unpack: observ = unpack(observpacked) + observpacked = getObservation(factor) + packtype = DFG.getTypeFromSerializationModule("Packed" * factor.fnctype) + return convert(convertStructType(packtype), observpacked) else - fullFactorData = nothing + rethrow() end - catch ex - io = IOBuffer() - showerror(io, ex, catch_backtrace()) - err = String(take!(io)) - msg = "Error while unpacking '$(factor.label)' as '$(factor.fnctype)', please check the unpacking/packing converters for this factor - \r\n$err" - error(msg) end - - metadata = JSON3.read(base64decode(factor.metadata), Dict{Symbol, DFG.SmallDataTypes}) - - if !isnothing(fullFactorData) && fullFactorData.fnc isa FactorOperationalMemory - workmem = fullFactorData.fnc - else - workmem = nothing - end - return FactorCompute( - factor.label, - Tuple(factor._variableOrderSymbols), - observation, - factor.state, - workmem; - solverData = fullFactorData, - nstime = Nanosecond(factor.nstime), - tags = Set(factor.tags), - solvable = factor.solvable, - timestamp = factor.timestamp, - id = factor.id, - smallData = metadata, - ) -end - -function unpackObservation(factor::FactorDFG) - #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes - observpacked = getObservation(factor) - #TODO change to unpack: observ = unpack(observpacked) - packtype = DFG.getTypeFromSerializationModule("Packed" * factor.fnctype) - return convert(convertStructType(packtype), observpacked) end packObservation(f::FactorCompute) = packObservation(getObservation(f)) function packObservation(observ::AbstractFactor) - packtype = convertPackedType(observ) - return convert(packtype, observ) + try + return pack(observ) + catch e + if e isa MethodError + Base.depwarn( + "$e\nPlease implement pack and unpack methods for the factor type '$(typeof(observ))'. + \nFalling back to deprecated convert method.", + :packObservation, + ) + packtype = convertPackedType(observ) + return convert(packtype, observ) + else + rethrow() + end + end end +# Deprecated check usefull? # packedFnc = fncStringToData(factor.fnctype, factor.data) +# Deprecated check usefull? # decodeType = getFactorOperationalMemoryType(dfg) +# Deprecated check usefull? # fullFactorData = decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) + function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) # @debug "DECODING factor type = '$(factor.fnctype)' for factor '$(factor.label)'" @@ -451,7 +410,7 @@ function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) local observation try - observation = unpackObservation(factor) + observation = unpackObservation(factor) #TODO maybe getObservation(factor) catch @error "Error while unpacking '$(factor.label)' as '$(factor.fnctype)', please check the unpacking/packing converters for this factor" rethrow() @@ -464,7 +423,6 @@ function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) Tuple(factor._variableOrderSymbols), factor.timestamp, Nanosecond(factor.nstime), - Ref{GenericFunctionNodeData}(), Ref(factor.solvable), getMetadata(factor), observation, diff --git a/test/testBlocks.jl b/test/testBlocks.jl index 1197768f..589efb8a 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -4,7 +4,6 @@ using Dates using Manifolds import Base: convert -import DistributedFactorGraphs: reconstFactorData # import DistributedFactorGraphs: getData, addData!, updateData!, deleteData! # Test InferenceVariable Types @@ -43,13 +42,14 @@ function Base.convert(::Type{PackedTestFunctorInferenceType1}, d::TestFunctorInf return PackedTestFunctorInferenceType1() end -function reconstFactorData( +function DFG.reconstFactorData( dfg::AbstractDFG, vo::AbstractVector, ::Type{TestFunctorInferenceType1}, d::PackedTestFunctorInferenceType1, ::String, ) + error("obsolete, TODO remove") return TestFunctorInferenceType1() end @@ -83,14 +83,15 @@ TestCCW{T}() where {T} = TestCCW(T()) Base.:(==)(a::TestCCW, b::TestCCW) = a.usrfnc! == b.usrfnc! DFG.getFactorOperationalMemoryType(par::NoSolverParams) = TestCCW -DFG.rebuildFactorMetadata!(dfg::AbstractDFG{NoSolverParams}, fac::FactorCompute) = fac +DFG.rebuildFactorWorkmem!(dfg::AbstractDFG{NoSolverParams}, fac::FactorCompute) = fac -function reconstFactorData( +function DFG.reconstFactorData( dfg::AbstractDFG, vo::AbstractVector, ::Type{<:DFG.FunctionNodeData{TestCCW{F}}}, d::DFG.PackedFunctionNodeData{<:AbstractPackedFactor}, ) where {F <: DFG.AbstractFactor} + error("obsolete, TODO remove") nF = convert(F, d.fnc) return DFG.FunctionNodeData( d.eliminated, From 6304e4d2fb5bc763accbb9afa9e32df2ab4ded9d Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 17 Jun 2025 17:09:16 +0200 Subject: [PATCH 08/14] rename to AbstractFactorObservation and FactorOperationalMemory -> FactorSolverCache --- src/Common.jl | 4 +- src/Deprecated.jl | 20 +++++--- src/DistributedFactorGraphs.jl | 6 +-- src/FileDFG/services/FileDFG.jl | 2 +- src/entities/DFGFactor.jl | 82 ++++++++++++++++----------------- src/services/AbstractDFG.jl | 14 +++--- src/services/CompareUtils.jl | 8 ++-- src/services/DFGFactor.jl | 20 ++++---- src/services/DFGVariable.jl | 2 +- src/services/Serialization.jl | 16 +++---- test/testBlocks.jl | 18 ++++---- 11 files changed, 100 insertions(+), 92 deletions(-) diff --git a/src/Common.jl b/src/Common.jl index 10fed638..25c667b8 100644 --- a/src/Common.jl +++ b/src/Common.jl @@ -3,10 +3,10 @@ _getmodule(t::T) where {T} = T.name.module _getname(t::T) where {T} = T.name.name -function convertPackedType(t::Union{T, Type{T}}) where {T <: AbstractFactor} +function convertPackedType(t::Union{T, Type{T}}) where {T <: AbstractFactorObservation} return getfield(_getmodule(t), Symbol("Packed$(_getname(t))")) end -function convertStructType(::Type{PT}) where {PT <: AbstractPackedFactor} +function convertStructType(::Type{PT}) where {PT <: AbstractPackedFactorObservation} # see #668 for expanded reasoning. PT may be ::UnionAll if the type is of template type. ptt = PT isa DataType ? PT.name.name : PT moduleName = PT isa DataType ? PT.name.module : Main diff --git a/src/Deprecated.jl b/src/Deprecated.jl index e96b83a4..0504867c 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -1,6 +1,14 @@ ## ================================================================================ ## Deprecated in v0.27 ##================================================================================= +export AbstractFactor +const AbstractFactor = AbstractFactorObservation + +export AbstractPackedFactor +const AbstractPackedFactor = AbstractPackedFactorObservation + +export FactorOperationalMemory +const FactorOperationalMemory = FactorSolverCache @deprecate getNeighborhood(args...; kwargs...) listNeighborhood(args...; kwargs...) @deprecate addBlob!(store::AbstractBlobstore, blobId::UUID, data, ::String) addBlob!( @@ -204,7 +212,7 @@ function FactorCompute( variableOrder::Union{Vector{Symbol}, Tuple}; observation = getFactorType(solverData), state::FactorState = FactorState(), - workmem::Base.RefValue{<:FactorOperationalMemory} = Ref{FactorOperationalMemory}(), + solvercache::Base.RefValue{<:FactorSolverCache} = Ref{FactorSolverCache}(), id::Union{UUID, Nothing} = nothing, smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), ) @@ -223,19 +231,19 @@ function FactorCompute( smallData, observation, state, - workmem, + solvercache, ) end function getSolverData(f::FactorCompute) return error( - "getSolverData(f::FactorCompute) is obsolete, use getState, getObservation, or getWorkmem instead", + "getSolverData(f::FactorCompute) is obsolete, use getState, getObservation, or getCache instead", ) end function setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) return error( - "setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) is obsolete, use setState!, or setWorkmem! instead", + "setSolverData!(f::FactorCompute, data::GenericFunctionNodeData) is obsolete, use setState!, or setCache! instead", ) end @@ -244,7 +252,7 @@ end skipVersionCheck, ) -@deprecate rebuildFactorMetadata!(args...; kwargs...) rebuildFactorWorkmem!( +@deprecate rebuildFactorMetadata!(args...; kwargs...) rebuildFactorCache!( args...; kwargs..., ) @@ -257,7 +265,7 @@ function decodePackedType( varOrder::AbstractVector{Symbol}, ::Type{T}, packeddata::GenericFunctionNodeData{PT}, -) where {T <: FactorOperationalMemory, PT} +) where {T <: FactorSolverCache, PT} error("decodePackedType is obsolete") # # TODO, to solve IIF 1424 diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 5557e3cf..f5f4ea15 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -248,9 +248,9 @@ export @format_str ##------------------------------------------------------------------------------ # Factor Data export GenericFunctionNodeData, PackedFunctionNodeData, FunctionNodeData -export AbstractFactor, AbstractPackedFactor +export AbstractFactorObservation, AbstractPackedFactorObservation export AbstractPrior, AbstractRelative, AbstractRelativeMinimize, AbstractManifoldMinimize -export FactorOperationalMemory +export FactorSolverCache # accessors export getVariableOrder @@ -286,7 +286,7 @@ export findClosestTimestamp, findVariableNearTimestamp # Serialization export packVariable, unpackVariable, packFactor, unpackFactor -export rebuildFactorWorkmem! +export rebuildFactorCache! export @defVariable # File import and export diff --git a/src/FileDFG/services/FileDFG.jl b/src/FileDFG/services/FileDFG.jl index 380c196c..718bcd94 100644 --- a/src/FileDFG/services/FileDFG.jl +++ b/src/FileDFG/services/FileDFG.jl @@ -206,7 +206,7 @@ function loadDFG!( # NOTE CREATES A NEW FactorCompute IF CCW TYPE CHANGES # @info "Rebuilding CCW's for the factors..." @showprogress 1 "Rebuilding factor working memory" for factor in factors - rebuildFactorWorkmem!(dfgLoadInto, factor) + rebuildFactorCache!(dfgLoadInto, factor) end end diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 3918c07c..e371d8c8 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -2,21 +2,21 @@ ## Abstract Types ##============================================================================== -abstract type AbstractFactor end -abstract type AbstractPackedFactor end +abstract type AbstractPackedFactorObservation end +abstract type AbstractFactorObservation end -abstract type AbstractPrior <: AbstractFactor end -abstract type AbstractRelative <: AbstractFactor end +abstract type AbstractPrior <: AbstractFactorObservation end +abstract type AbstractRelative <: AbstractFactorObservation end abstract type AbstractRelativeMinimize <: AbstractRelative end abstract type AbstractManifoldMinimize <: AbstractRelative end -# NOTE DF, Convolution is IIF idea, but DFG should know about "FactorOperationalMemory" -# DF, IIF.CommonConvWrapper <: FactorOperationalMemory # +# NOTE DF, Convolution is IIF idea, but DFG should know about "FactorSolverCache" +# DF, IIF.CommonConvWrapper <: FactorSolverCache # # NOTE was `<: Function` as unnecessary -abstract type FactorOperationalMemory end +abstract type FactorSolverCache end # TODO to be removed from DFG, -# we can add to IIF or have IIF.CommonConvWrapper <: FactorOperationalMemory directly -# abstract type ConvolutionObject <: FactorOperationalMemory end +# we can add to IIF or have IIF.CommonConvWrapper <: FactorSolverCache directly +# abstract type ConvolutionObject <: FactorSolverCache end ##============================================================================== ## GenericFunctionNodeData @@ -29,13 +29,13 @@ Notes - S::Symbol Designing (WIP) -- T <: Union{FactorOperationalMemory, AbstractPackedFactor} -- in IIF.CCW{T <: DFG.AbstractFactor} -- in DFG.AbstractRelativeMinimize <: AbstractFactor +- T <: Union{FactorSolverCache, AbstractPackedFactorObservation} +- in IIF.CCW{T <: DFG.AbstractFactorObservation} +- in DFG.AbstractRelativeMinimize <: AbstractFactorObservation - in Main.SomeFactor <: AbstractRelativeMinimize """ Base.@kwdef mutable struct GenericFunctionNodeData{ - T <: Union{<:AbstractPackedFactor, <:AbstractFactor, <:FactorOperationalMemory}, + T <: Union{<:AbstractPackedFactorObservation, <:AbstractFactorObservation, <:FactorSolverCache}, } eliminated::Bool = false potentialused::Bool = false @@ -59,9 +59,9 @@ end inflation::Float64 = 0.0 end -# TODO should we move non FactorOperationalMemory to FactorCompute: +# TODO should we move non FactorSolverCache to FactorCompute: # fnc, multihypo, nullhypo, inflation ? -# that way we split solverData <: FactorOperationalMemory and constants +# that way we split solverData <: FactorSolverCache and constants # TODO see if above ever changes? ## Constructors @@ -70,19 +70,19 @@ end ## PackedFunctionNodeData and FunctionNodeData const PackedFunctionNodeData{T} = - GenericFunctionNodeData{T} where {T <: AbstractPackedFactor} + GenericFunctionNodeData{T} where {T <: AbstractPackedFactorObservation} function PackedFunctionNodeData(args...; kw...) return PackedFunctionNodeData{typeof(args[4])}(args...; kw...) end const FunctionNodeData{T} = GenericFunctionNodeData{ T, -} where {T <: Union{<:AbstractFactor, <:FactorOperationalMemory}} +} where {T <: Union{<:AbstractFactorObservation, <:FactorSolverCache}} FunctionNodeData(args...; kw...) = FunctionNodeData{typeof(args[4])}(args...; kw...) -# PackedFunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: AbstractPackedFactor = +# PackedFunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: AbstractPackedFactorObservation = # GenericFunctionNodeData{T}(x2, x3, x4, x6, multihypo, certainhypo, x9) -# FunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: Union{AbstractFactor, FactorOperationalMemory} = +# FunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: Union{AbstractFactorObservation, FactorSolverCache} = # GenericFunctionNodeData{T}(x2, x3, x4, x6, multihypo, certainhypo, x9) ##============================================================================== @@ -183,24 +183,24 @@ $(TYPEDEF) Abstract parent type for all InferenceTypes, which are the observation functions inside of factors. """ -abstract type InferenceType <: AbstractPackedFactor end +abstract type InferenceType <: AbstractPackedFactorObservation end -#TODO deprecate InferenceType in favor of AbstractPackedFactor v0.26 +#TODO deprecate InferenceType in favor of AbstractPackedFactorObservation v0.26 # this is the GenericFunctionNodeData for packed types #TODO deprecate FactorData in favor of FactorState (with no more distinction between packed and compute) -const FactorData = PackedFunctionNodeData{AbstractPackedFactor} +const FactorData = PackedFunctionNodeData{AbstractPackedFactorObservation} # Packed Factor constructor function assembleFactorName(xisyms::Union{Vector{String}, Vector{Symbol}}) return Symbol(xisyms..., "_f", randstring(4)) end -getFncTypeName(fnc::AbstractPackedFactor) = split(string(typeof(fnc)), ".")[end] +getFncTypeName(fnc::AbstractPackedFactorObservation) = split(string(typeof(fnc)), ".")[end] function FactorDFG( xisyms::Vector{Symbol}, - fnc::AbstractPackedFactor; + fnc::AbstractPackedFactorObservation; multihypo::Vector{Float64} = Float64[], nullhypo::Float64 = 0.0, solvable::Int = 1, @@ -249,7 +249,7 @@ DevNotes Fields: $(TYPEDFIELDS) """ -Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor +Base.@kwdef struct FactorCompute{FT <: AbstractFactorObservation, N} <: AbstractDFGFactor """The ID for the factor""" id::Union{UUID, Nothing} = nothing #TODO deprecate id """Factor label, e.g. :x1f1. @@ -281,9 +281,9 @@ Base.@kwdef struct FactorCompute{FT <: AbstractFactor, N} <: AbstractDFGFactor Accessors: [`getFactorState`](@ref)""" state::FactorState """Temporary, non-persistent memory used internally by the solver for intermediate numerical computations and buffers. - `workmem` is lazily allocated and only used during factor operations; it is not serialized or retained after solving. - Accessors: [`getWorkmem`](@ref), [`setWorkmem!`](@ref)""" - workmem::Base.RefValue{<:FactorOperationalMemory} #TODO easy of use vs. performance as container is abstract in any case. + `solvercache` is lazily allocated and only used during factor operations; it is not serialized or retained after solving. + Accessors: [`getCache`](@ref), [`setCache!`](@ref)""" + solvercache::Base.RefValue{<:FactorSolverCache} #TODO easy of use vs. performance as container is abstract in any case. end ##------------------------------------------------------------------------------ @@ -293,9 +293,9 @@ end function FactorCompute( label::Symbol, variableOrder::Union{Vector{Symbol}, Tuple}, - observation::AbstractFactor, + observation::AbstractFactorObservation, state::FactorState = FactorState(), - _workmem = nothing; + cache = nothing; tags::Set{Symbol} = Set{Symbol}(), timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), solvable::Int = 1, @@ -308,10 +308,10 @@ function FactorCompute( Base.depwarn("`FactorCompute` solverData is deprecated", :FactorCompute) end - if isnothing(_workmem) - workmem = Ref{FactorOperationalMemory}() + if isnothing(cache) + solvercache = Ref{FactorSolverCache}() else - workmem = Ref(_workmem) + solvercache = Ref(cache) end return FactorCompute( @@ -325,7 +325,7 @@ function FactorCompute( smallData, observation, state, - workmem, + solvercache, ) end @@ -341,7 +341,7 @@ function FactorCompute( smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), ) Base.depwarn( - "`FactorCompute` constructor with `GenericFunctionNodeData` is deprecated. observation, state, and workmem should be provided explicitly.", + "`FactorCompute` constructor with `GenericFunctionNodeData` is deprecated. observation, state, and solvercache should be provided explicitly.", :FactorCompute, ) observation = getFactorType(solverData) @@ -355,10 +355,10 @@ function FactorCompute( solverData.inflation, ) - if solverData.fnc isa FactorOperationalMemory - workmem = solverData.fnc + if solverData.fnc isa FactorSolverCache + solvercache = solverData.fnc else - workmem = nothing + solvercache = nothing end return FactorCompute( @@ -366,7 +366,7 @@ function FactorCompute( Tuple(variableOrder), observation, state, - workmem; + solvercache; id, timestamp, nstime, @@ -381,7 +381,7 @@ function Base.getproperty(x::FactorCompute, f::Symbol) getfield(x, f)[] elseif f == :solverData error( - "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getState` or `getWorkmem` instead.", + "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getState` or `getCache` instead.", ) elseif f == :_variableOrderSymbols [getfield(x, f)...] @@ -395,7 +395,7 @@ function Base.setproperty!(x::FactorCompute, f::Symbol, val) getfield(x, f)[] = val elseif f == :solverData error( - "`solverData` is obsolete in `FactorCompute`. Use `Observation`, `State` or `Workmem` instead.", + "`solverData` is obsolete in `FactorCompute`. Use `Observation`, `State` or `Cache` instead.", ) else setfield!(x, f, val) diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl index 6d54727e..afb847a1 100644 --- a/src/services/AbstractDFG.jl +++ b/src/services/AbstractDFG.jl @@ -100,11 +100,11 @@ getSolverParams(dfg::AbstractDFG) = dfg.solverParams """ $(SIGNATURES) -Method must be overloaded by the user for Serialization to work. E.g. IncrementalInference uses `CommonConvWrapper <: FactorOperationalMemory`. +Method must be overloaded by the user for Serialization to work. E.g. IncrementalInference uses `CommonConvWrapper <: FactorSolverCache`. """ function getFactorOperationalMemoryType(dummy) return error( - "Please extend your workspace with function getFactorOperationalMemoryType(<:AbstractParams) for your usecase, e.g. IncrementalInference uses `CommonConvWrapper <: FactorOperationalMemory`", + "Please extend your workspace with function getFactorOperationalMemoryType(<:AbstractParams) for your usecase, e.g. IncrementalInference uses `CommonConvWrapper <: FactorSolverCache`", ) end function getFactorOperationalMemoryType(dfg::AbstractDFG) @@ -116,12 +116,12 @@ end Method must be overloaded by the user for Serialization to work. """ -function rebuildFactorWorkmem!( +function rebuildFactorCache!( dfg::AbstractDFG{<:AbstractParams}, factor::AbstractDFGFactor, neighbors = [], ) - return error("rebuildFactorWorkmem! is not implemented for $(typeof(dfg))") + return error("rebuildFactorCache! is not implemented for $(typeof(dfg))") end """ @@ -769,7 +769,7 @@ function ls(dfg::G, ::Type{T}) where {G <: AbstractDFG, T <: InferenceVariable} return map(x -> x.label, vxx) end -function ls(dfg::G, ::Type{T}) where {G <: AbstractDFG, T <: AbstractFactor} +function ls(dfg::G, ::Type{T}) where {G <: AbstractDFG, T <: AbstractFactorObservation} xx = getFactors(dfg) names = typeof.(getFactorType.(xx)) .|> nameof vxx = view(xx, names .== Symbol(T)) @@ -785,7 +785,7 @@ Example, list all the Point2Point2 factors in the factor graph `dfg`: Notes - Return `Vector{Symbol}` """ -function lsf(dfg::G, ::Type{T}) where {G <: AbstractDFG, T <: AbstractFactor} +function lsf(dfg::G, ::Type{T}) where {G <: AbstractDFG, T <: AbstractFactorObservation} return ls(dfg, T) end @@ -1283,7 +1283,7 @@ function existsPathOfFactorsType( dfg::AbstractDFG, from::Symbol, to::Symbol, - ftype::AbstractFactor, + ftype::AbstractFactorObservation, ) return error("WIP") end diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index f8c01048..92f02d15 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -12,9 +12,9 @@ implement compare if needed. =# # ==(a::InferenceVariable,b::InferenceVariable) = typeof(a) == typeof(b) && a.dims == b.dims && a.manifolds == b.manifolds -==(a::FactorOperationalMemory, b::FactorOperationalMemory) = typeof(a) == typeof(b) +==(a::FactorSolverCache, b::FactorSolverCache) = typeof(a) == typeof(b) -==(a::AbstractFactor, b::AbstractFactor) = typeof(a) == typeof(b) +==(a::AbstractFactorObservation, b::AbstractFactorObservation) = typeof(a) == typeof(b) # Generate compares automatically for all in this union const GeneratedCompareUnion = Union{ @@ -34,7 +34,7 @@ const GeneratedCompareUnion = Union{ } @generated function ==(x::T, y::T) where {T <: GeneratedCompareUnion} - ignored = [:workmem, :solverData] + ignored = [:solvercache, :solverData] return mapreduce( n -> :(x.$n == y.$n), (a, b) -> :($a && $b), @@ -321,7 +321,7 @@ function compareFactor( :attributes, :solverData, :observation, - :workmem, + :solvercache, :_variableOrderSymbols, :_gradients, ], diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index fcff8348..5ab04d47 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -32,12 +32,12 @@ Return user factor type from factor graph identified by label `::Symbol`. Notes - Replaces older `getfnctype`. """ -function getFactorType(data::GenericFunctionNodeData{<:FactorOperationalMemory}) +function getFactorType(data::GenericFunctionNodeData{<:FactorSolverCache}) #TODO deprecated in v0.27 Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) return data.fnc.usrfnc! end -function getFactorType(data::GenericFunctionNodeData{<:AbstractFactor}) +function getFactorType(data::GenericFunctionNodeData{<:AbstractFactorObservation}) #TODO deprecated in v0.27 Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) return data.fnc @@ -59,14 +59,14 @@ function getObservation(f::FactorDFG) # return packtype(JSON3.read(f.observJSON)) end -function getWorkmem(f::FactorCompute) - if isassigned(f.workmem) - return f.workmem[] +function getCache(f::FactorCompute) + if isassigned(f.solvercache) + return f.solvercache[] else return nothing end end -setWorkmem!(f::FactorCompute, workmem::FactorOperationalMemory) = f.workmem[] = workmem +setCache!(f::FactorCompute, solvercache::FactorSolverCache) = f.solvercache[] = solvercache """ $SIGNATURES @@ -107,7 +107,7 @@ function Base.convert(::Type{<:PackedSamplableBelief}, nt::Union{NamedTuple, JSO end """ - @defFactorType StructName factortype<:AbstractFactor manifolds<:AbstractManifold + @defFactorType StructName factortype<:AbstractFactorObservation manifolds<:AbstractManifold A macro to create a new factor function with name `StructName` and manifold. Note that the `manifold` is an object and *must* be a subtype of `ManifoldsBase.AbstractManifold`. @@ -127,16 +127,16 @@ macro defFactorType(structname, factortype, manifold) string($manifold) * ") is not an `AbstractManifold`" - @assert ($factortype <: AbstractFactor) "@defFactorType factortype (" * + @assert ($factortype <: AbstractFactorObservation) "@defFactorType factortype (" * string($factortype) * - ") is not an `AbstractFactor`" + ") is not an `AbstractFactorObservation`" Base.@__doc__ struct $structname{T} <: $factortype Z::T end #TODO should this be $packedstructname{T <: PackedSamplableBelief} - Base.@__doc__ struct $packedstructname <: AbstractPackedFactor + Base.@__doc__ struct $packedstructname <: AbstractPackedFactorObservation Z::PackedSamplableBelief end diff --git a/src/services/DFGVariable.jl b/src/services/DFGVariable.jl index fdfae0ea..bdc071ce 100644 --- a/src/services/DFGVariable.jl +++ b/src/services/DFGVariable.jl @@ -123,7 +123,7 @@ Interface function to return the `<:ManifoldsBase.AbstractManifold` object of `v """ getManifold(::T) where {T <: InferenceVariable} = getManifold(T) getManifold(vari::VariableCompute) = getVariableType(vari) |> getManifold -# covers both <:InferenceVariable and <:AbstractFactor +# covers both <:InferenceVariable and <:AbstractFactorObservation getManifold(dfg::AbstractDFG, lbl::Symbol) = getManifold(dfg[lbl]) """ diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 99297f3f..7740d936 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -251,7 +251,7 @@ VariableDFG(v::VariableCompute) = packVariable(v) ## Factor Packing and unpacking ##============================================================================== -function _packSolverData(f::FactorCompute, fnctype::AbstractFactor) +function _packSolverData(f::FactorCompute, fnctype::AbstractFactorObservation) # Base.depwarn( "_packSolverData is deprecated, use seperate packing of observation #TODO", @@ -294,7 +294,7 @@ end packFactor(f::FactorDFG) = f -function fncStringToData(packtype::Type{<:AbstractPackedFactor}, data::String) +function fncStringToData(packtype::Type{<:AbstractPackedFactorObservation}, data::String) # Read string as JSON object to use as kwargs fncData = JSON3.read(if data[1] == '{' @@ -319,7 +319,7 @@ function fncStringToData(packtype::Type{<:AbstractPackedFactor}, data::String) return packed end -function fncStringToData(packtype::Type{<:AbstractPackedFactor}, data::NamedTuple) +function fncStringToData(packtype::Type{<:AbstractPackedFactorObservation}, data::NamedTuple) return error( "Who is calling deserialize factor with NamedTuple, likely JSON3 somewhere", ) @@ -328,13 +328,13 @@ end function fncStringToData( ::Type{T}, data::PackedFunctionNodeData{T}, -) where {T <: AbstractPackedFactor} +) where {T <: AbstractPackedFactorObservation} return data end function fncStringToData( fncType::String, data::PackedFunctionNodeData{T}, -) where {T <: AbstractPackedFactor} +) where {T <: AbstractPackedFactorObservation} packtype = DFG.getTypeFromSerializationModule("Packed" * fncType) if packtype == T data @@ -345,7 +345,7 @@ function fncStringToData( end end -function fncStringToData(fncType::String, data::T) where {T <: AbstractPackedFactor} +function fncStringToData(fncType::String, data::T) where {T <: AbstractPackedFactorObservation} packtype = DFG.getTypeFromSerializationModule("Packed" * fncType) if packtype == T # || T <: packtype data @@ -381,7 +381,7 @@ function unpackObservation(factor::FactorDFG) end packObservation(f::FactorCompute) = packObservation(getObservation(f)) -function packObservation(observ::AbstractFactor) +function packObservation(observ::AbstractFactorObservation) try return pack(observ) catch e @@ -427,7 +427,7 @@ function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) getMetadata(factor), observation, factor.state, - Ref{FactorOperationalMemory}(), + Ref{FactorSolverCache}(), ) end diff --git a/test/testBlocks.jl b/test/testBlocks.jl index 589efb8a..78273771 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -32,7 +32,7 @@ struct TestAbstractPrior <: AbstractPrior end # struct TestAbstractRelativeFactor <: AbstractRelativeRoots end struct TestAbstractRelativeFactorMinimize <: AbstractRelativeMinimize end -Base.@kwdef struct PackedTestFunctorInferenceType1 <: AbstractPackedFactor +Base.@kwdef struct PackedTestFunctorInferenceType1 <: AbstractPackedFactorObservation s::String = "" end # PackedTestFunctorInferenceType1() = PackedTestFunctorInferenceType1("") @@ -59,7 +59,7 @@ function Base.convert(::Type{TestFunctorInferenceType1}, d::PackedTestFunctorInf return TestFunctorInferenceType1() end -Base.@kwdef struct PackedTestAbstractPrior <: AbstractPackedFactor +Base.@kwdef struct PackedTestAbstractPrior <: AbstractPackedFactorObservation s::String = "" end # PackedTestAbstractPrior() = PackedTestAbstractPrior("") @@ -74,7 +74,7 @@ function Base.convert(::Type{TestAbstractPrior}, d::PackedTestAbstractPrior) return TestAbstractPrior() end -struct TestCCW{T <: AbstractFactor} <: FactorOperationalMemory +struct TestCCW{T <: AbstractFactorObservation} <: FactorSolverCache usrfnc!::T end @@ -83,14 +83,14 @@ TestCCW{T}() where {T} = TestCCW(T()) Base.:(==)(a::TestCCW, b::TestCCW) = a.usrfnc! == b.usrfnc! DFG.getFactorOperationalMemoryType(par::NoSolverParams) = TestCCW -DFG.rebuildFactorWorkmem!(dfg::AbstractDFG{NoSolverParams}, fac::FactorCompute) = fac +DFG.rebuildFactorCache!(dfg::AbstractDFG{NoSolverParams}, fac::FactorCompute) = fac function DFG.reconstFactorData( dfg::AbstractDFG, vo::AbstractVector, ::Type{<:DFG.FunctionNodeData{TestCCW{F}}}, - d::DFG.PackedFunctionNodeData{<:AbstractPackedFactor}, -) where {F <: DFG.AbstractFactor} + d::DFG.PackedFunctionNodeData{<:AbstractPackedFactorObservation}, +) where {F <: DFG.AbstractFactorObservation} error("obsolete, TODO remove") nF = convert(F, d.fnc) return DFG.FunctionNodeData( @@ -108,8 +108,8 @@ end function Base.convert( ::Type{DFG.PackedFunctionNodeData{P}}, - d::DFG.FunctionNodeData{<:FactorOperationalMemory}, -) where {P <: AbstractPackedFactor} + d::DFG.FunctionNodeData{<:FactorSolverCache}, +) where {P <: AbstractPackedFactorObservation} return DFG.PackedFunctionNodeData( d.eliminated, d.potentialused, @@ -130,7 +130,7 @@ end #test Specific definitions # struct TestInferenceVariable1 <: InferenceVariable end # struct TestInferenceVariable2 <: InferenceVariable end -# struct TestFunctorInferenceType1 <: AbstractFactor end +# struct TestFunctorInferenceType1 <: AbstractFactorObservation end # NOTE see note in AbstractDFG.jl setSolverParams! struct GeenSolverParams <: AbstractParams end From b7b90f971860f38a0958437a71500e30102f8677 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 17 Jun 2025 17:12:06 +0200 Subject: [PATCH 09/14] fix test and factor solverData related issues --- src/Deprecated.jl | 19 +++++++++++++++++++ src/DistributedFactorGraphs.jl | 3 ++- src/FileDFG/services/FileDFG.jl | 30 +++++++----------------------- src/entities/DFGFactor.jl | 6 ++++-- src/services/CommonAccessors.jl | 2 +- src/services/DFGFactor.jl | 5 ++++- src/services/Serialization.jl | 24 ------------------------ test/iifCompareTests.jl | 2 +- test/iifInterfaceTests.jl | 10 ++++++---- test/runtests.jl | 2 +- test/testBlocks.jl | 19 ++++++++----------- 11 files changed, 53 insertions(+), 69 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 0504867c..2ec957cb 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -235,6 +235,8 @@ function FactorCompute( ) end +export getSolverData, setSolverData! + function getSolverData(f::FactorCompute) return error( "getSolverData(f::FactorCompute) is obsolete, use getState, getObservation, or getCache instead", @@ -278,6 +280,23 @@ function decodePackedType( return factordata end +export _packSolverData +function _packSolverData(f::FactorCompute, fnctype::AbstractFactorObservation) + # + error("_packSolverData is deprecated, use seperate packing of observation #TODO") + packtype = convertPackedType(fnctype) + try + packed = convert(PackedFunctionNodeData{packtype}, getSolverData(f)) #TODO getSolverData + packedJson = packed + return packedJson + catch ex + io = IOBuffer() + showerror(io, ex, catch_backtrace()) + err = String(take!(io)) + msg = "Error while packing '$(f.label)' as '$fnctype', please check the unpacking/packing converters for this factor - \r\n$err" + error(msg) + end +end ## ================================================================================ ## Deprecated in v0.25 ##================================================================================= diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index f5f4ea15..9f09a598 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -166,7 +166,8 @@ export InferenceVariable export getSolverDataDict, setSolverData! export getVariableType, getVariableTypeName -export getSolverData +export getObservation +export getState export getVariableType diff --git a/src/FileDFG/services/FileDFG.jl b/src/FileDFG/services/FileDFG.jl index 718bcd94..84a71f33 100644 --- a/src/FileDFG/services/FileDFG.jl +++ b/src/FileDFG/services/FileDFG.jl @@ -124,7 +124,7 @@ function loadDFG!( if unzip Base.mkpath(loaddir) folder = joinpath(loaddir, filename) - @info "loadDFG! detected a gzip $dstname -- unpacking via $loaddir now..." + @debug "loadDFG! detected a gzip $dstname -- unpacking via $loaddir now..." Base.rm(folder; recursive = true, force = true) # unzip the tar file tar_gz = open(dstname) @@ -167,18 +167,11 @@ function loadDFG!( variables = @showprogress 1 "loading variables" asyncmap(varFiles) do varFile jstr = read("$varFolder/$varFile", String) packedvar = JSON3.read(jstr, VariableDFG) - if usePackedVariable - return packedvar - else - return unpackVariable(packedvar) - end + v = usePackedVariable ? packedvar : unpackVariable(packedvar) + addVariable!(dfgLoadInto, v) end @info "Loaded $(length(variables)) variables"#- $(map(v->v.label, variables))" - @showprogress 1 "Inserting variables into graph" map( - v -> addVariable!(dfgLoadInto, v), - variables, - ) usePackedFactor = isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) == FactorDFG @@ -187,25 +180,16 @@ function loadDFG!( factors = @showprogress 1 "loading factors" asyncmap(factorFiles) do factorFile jstr = read("$factorFolder/$factorFile", String) packedfact = JSON3.read(jstr, FactorDFG) - if usePackedFactor - return packedfact - else - return unpackFactor(packedfact) - end + f = usePackedFactor ? packedfact : unpackFactor(packedfact) + addFactor!(dfgLoadInto, f) end @info "Loaded $(length(factors)) factors"# - $(map(f->f.label, factors))" - # # Adding factors - @showprogress 1 "Inserting factors into graph" map( - f -> addFactor!(dfgLoadInto, f), - factors, - ) if isa(dfgLoadInto, GraphsDFG) && getTypeDFGFactors(dfgLoadInto) != FactorDFG # Finally, rebuild the CCW's for the factors to completely reinflate them - # NOTE CREATES A NEW FactorCompute IF CCW TYPE CHANGES - # @info "Rebuilding CCW's for the factors..." - @showprogress 1 "Rebuilding factor working memory" for factor in factors + # NOTE CREATES A NEW FactorCompute IF CCW TYPE CHANGES + @showprogress 1 "Rebuilding factor solver cache" for factor in factors rebuildFactorCache!(dfgLoadInto, factor) end end diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index e371d8c8..951de48c 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -114,6 +114,7 @@ Base.@kwdef struct FactorDFG <: AbstractDFGFactor nstime::String fnctype::String solvable::Int + data::Union{Nothing, String} #TODO deprecate data completely, left as a bridge to old serialization structure metadata::String _version::String = string(_getDFGVersion()) state::FactorState @@ -143,8 +144,8 @@ function FactorDFG( data::Union{Nothing, String}, metadata::String, _version::String, - state::Union{Nothing, FactorState}, - observJSON::Union{Nothing, String}, + state::Union{Nothing, FactorState} = nothing, + observJSON::Union{Nothing, String} = nothing, ) if isnothing(state) || isnothing(observJSON) fd = JSON3.read(data) @@ -168,6 +169,7 @@ function FactorDFG( nstime, fnctype, solvable, + nothing, #TODO deprecate data completely metadata, _version, state, diff --git a/src/services/CommonAccessors.jl b/src/services/CommonAccessors.jl index 856df9f2..9b41da99 100644 --- a/src/services/CommonAccessors.jl +++ b/src/services/CommonAccessors.jl @@ -165,7 +165,7 @@ function getSolveInProgress( end end # Factor - return getSolverData(var).solveInProgress + return getState(var).solveInProgress end #TODO missing set solveInProgress and graph level accessor diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 5ab04d47..d4add7cd 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -59,6 +59,8 @@ function getObservation(f::FactorDFG) # return packtype(JSON3.read(f.observJSON)) end +getObservation(dfg::AbstractDFG, lbl::Symbol) = getObservation(getFactor(dfg, lbl)) + function getCache(f::FactorCompute) if isassigned(f.solvercache) return f.solvercache[] @@ -187,7 +189,8 @@ function setTimestamp(f::FactorCompute, ts::ZonedDateTime) return FactorCompute( f.label, getfield(f, :_variableOrderSymbols), - f.solverData; + f.observation, + f.state; timestamp = ts, nstime = f.nstime, tags = f.tags, diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 7740d936..af29cf90 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -1,7 +1,3 @@ - -# TODO dev and debugging, used by some of the DFG drivers -export _packSolverData - ## Version checking #NOTE fixed really bad function but kept similar as fallback #TODO upgrade to use pkgversion(m::Module) function _getDFGVersion() @@ -251,26 +247,6 @@ VariableDFG(v::VariableCompute) = packVariable(v) ## Factor Packing and unpacking ##============================================================================== -function _packSolverData(f::FactorCompute, fnctype::AbstractFactorObservation) - # - Base.depwarn( - "_packSolverData is deprecated, use seperate packing of observation #TODO", - :_packSolverData, - ) - packtype = convertPackedType(fnctype) - try - packed = convert(PackedFunctionNodeData{packtype}, getSolverData(f)) #TODO getSolverData - packedJson = packed - return packedJson - catch ex - io = IOBuffer() - showerror(io, ex, catch_backtrace()) - err = String(take!(io)) - msg = "Error while packing '$(f.label)' as '$fnctype', please check the unpacking/packing converters for this factor - \r\n$err" - error(msg) - end -end - # returns FactorDFG function packFactor(f::FactorCompute) fnctype = getObservation(f) diff --git a/test/iifCompareTests.jl b/test/iifCompareTests.jl index dfaac6b2..9daf5e1c 100644 --- a/test/iifCompareTests.jl +++ b/test/iifCompareTests.jl @@ -75,7 +75,7 @@ using Test skip = [:fullvariables], ) - @test !compareSimilarFactors(fg, fg2; skipsamples = true, skipcompute = false) + @test_broken !compareSimilarFactors(fg, fg2; skipsamples = true, skipcompute = false) @test compareFactorGraphs( fg, diff --git a/test/iifInterfaceTests.jl b/test/iifInterfaceTests.jl index 151f9390..2ac66208 100644 --- a/test/iifInterfaceTests.jl +++ b/test/iifInterfaceTests.jl @@ -140,9 +140,10 @@ end # @test @test_deprecated getVariableIds(dfg) == listVariables(dfg) # @test @test_deprecated getFactorIds(dfg) == listFactors(dfg) - @test getFactorType(f1.solverData) === f1.solverData.fnc.usrfnc! - @test getFactorType(f1) === f1.solverData.fnc.usrfnc! - @test getFactorType(dfg, :abf1) === f1.solverData.fnc.usrfnc! + @test getObservation(dfg, :abf1) === f1.observation + @test getObservation(f1) === f1.observation + @test getFactorType(f1) === f1.observation + @test getFactorType(dfg, :abf1) === f1.observation @test !isPrior(dfg, :abf1) # f1 is not a prior @test lsfPriors(dfg) == [] @@ -199,7 +200,8 @@ end @test getLabel(f1) == f1.label @test getTags(f1) == f1.tags - @test getSolverData(f1) == f1.solverData + @test getState(f1) === f1.state + @test getObservation(f1) === f1.observation @test getSolverParams(dfg) !== nothing @test setSolverParams!(dfg, getSolverParams(dfg)) == getSolverParams(dfg) diff --git a/test/runtests.jl b/test/runtests.jl index 3ccaa549..b9cdcb01 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -73,7 +73,7 @@ if get(ENV, "IIF_TEST", "true") == "true" apis = Vector{AbstractDFG}() push!( apis, - GraphsDFG(; solverParams = SolverParams(), userLabel = "test@navability.io"), + GraphsDFG(; solverParams = SolverParams()), ) for api in apis diff --git a/test/testBlocks.jl b/test/testBlocks.jl index 78273771..ebe4c6de 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -414,11 +414,11 @@ function DFGFactorSCA() @test getSolvable(f1) == 0 - @test getSolverData(f1) === f1.solverData + @test getObservation(f1) === f1.observation @test getVariableOrder(f1) == [:a, :b] - getSolverData(f1).solveInProgress = 1 + getState(f1).solveInProgress = 1 @test setSolvable!(f1, 1) == 1 #TODO These 2 function are equivelent @@ -444,9 +444,6 @@ function DFGFactorSCA() @test setSolvable!(f1, 1) == 1 @test getSolvable(f1) == 1 - #TODO don't know if this should be used, added for completeness, it just wastes the gc's time - @test setSolverData!(f1, deepcopy(gfnd)) == gfnd - # create f0 here for a later timestamp f0 = FactorCompute(:af1, [:a], gfnd_prior; tags = Set([:PRIOR])) @@ -495,7 +492,8 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) f2_mod = FactorCompute( f2.label, (:a,), - f2.solverData; + f2.observation, + f2.state; timestamp = f2.timestamp, nstime = f2.nstime, tags = f2.tags, @@ -1166,9 +1164,9 @@ function testGroup!(fg, v1, v2, f0, f1) # TODO Mabye implement IIF type here # Requires IIF or a type in IIF - @test getFactorType(f1.solverData) === f1.solverData.fnc.usrfnc! - @test getFactorType(f1) === f1.solverData.fnc.usrfnc! - @test getFactorType(fg, :abf1) === f1.solverData.fnc.usrfnc! + @test getObservation(f1) === f1.observation + @test getFactorType(f1) === f1.observation + @test getFactorType(fg, :abf1) === f1.observation @test isPrior(fg, :af1) # if f1 is prior @test lsfPriors(fg) == [:af1] @@ -1850,10 +1848,9 @@ function FileDFGTestBlock(testDFGAPI; kwargs...) mergeVariable!(dfg, v4) f45 = getFactor(dfg, :x4x5f1) - fsd = f45.solverData + fsd = getState(f45) # set some factor solver data push!(fsd.certainhypo, 2) - push!(fsd.edgeIDs, 3) fsd.eliminated = true push!(fsd.multihypo, 4.0) fsd.nullhypo = 5.0 From 59d25bc4bc29a66e8f47d8c704e20a6aa98e6492 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 17 Jun 2025 21:21:17 +0200 Subject: [PATCH 10/14] deprecate GenericFunctionNodeData and related --- src/Deprecated.jl | 107 +++++++++++++++++++++++++++++++- src/DistributedFactorGraphs.jl | 2 - src/FileDFG/services/FileDFG.jl | 4 +- src/entities/DFGFactor.jl | 105 +------------------------------ src/services/CompareUtils.jl | 34 ---------- src/services/DFGFactor.jl | 19 +----- src/services/Serialization.jl | 69 -------------------- test/compareTests.jl | 37 +++-------- test/fileDFGTests.jl | 5 +- test/plottingTest.jl | 2 +- test/runtests.jl | 4 +- test/testBlocks.jl | 27 +++----- 12 files changed, 139 insertions(+), 276 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 2ec957cb..2d1b5d04 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -1,7 +1,7 @@ ## ================================================================================ ## Deprecated in v0.27 ##================================================================================= -export AbstractFactor +export AbstractFactor const AbstractFactor = AbstractFactorObservation export AbstractPackedFactor @@ -202,6 +202,37 @@ function updateVariableSolverData!( end end +## factor refactor deprecations +""" +$(TYPEDEF) + +Notes +- S::Symbol + +Designing (WIP) +- T <: Union{FactorSolverCache, AbstractPackedFactorObservation} +- in IIF.CCW{T <: DFG.AbstractFactorObservation} +- in DFG.AbstractRelativeMinimize <: AbstractFactorObservation +- in Main.SomeFactor <: AbstractRelativeMinimize +""" +Base.@kwdef mutable struct GenericFunctionNodeData{ + T <: Union{ + <:AbstractPackedFactorObservation, + <:AbstractFactorObservation, + <:FactorSolverCache, + }, +} + eliminated::Bool = false + potentialused::Bool = false + edgeIDs::Vector{Int} = Int[] + fnc::T + multihypo::Vector{Float64} = Float64[] # TODO re-evaluate after refactoring w #477 + certainhypo::Vector{Int} = Int[] + nullhypo::Float64 = 0.0 + solveInProgress::Int = 0 + inflation::Float64 = 0.0 +end + function FactorCompute( label::Symbol, timestamp::Union{DateTime, ZonedDateTime}, @@ -297,6 +328,80 @@ function _packSolverData(f::FactorCompute, fnctype::AbstractFactorObservation) error(msg) end end + +export GenericFunctionNodeData, PackedFunctionNodeData, FunctionNodeData + +const PackedFunctionNodeData{T} = + GenericFunctionNodeData{T} where {T <: AbstractPackedFactorObservation} +function PackedFunctionNodeData(args...; kw...) + error("PackedFunctionNodeData is obsolete") + return PackedFunctionNodeData{typeof(args[4])}(args...; kw...) +end + +const FunctionNodeData{T} = GenericFunctionNodeData{ + T, +} where {T <: Union{<:AbstractFactorObservation, <:FactorSolverCache}} +FunctionNodeData(args...; kw...) = FunctionNodeData{typeof(args[4])}(args...; kw...) + +# this is the GenericFunctionNodeData for packed types +#TODO deprecate FactorData in favor of FactorState (with no more distinction between packed and compute) +const FactorData = PackedFunctionNodeData{AbstractPackedFactorObservation} + +function FactorCompute( + label::Symbol, + variableOrder::Union{Vector{Symbol}, Tuple}, + solverData::GenericFunctionNodeData; + tags::Set{Symbol} = Set{Symbol}(), + timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), + solvable::Int = 1, + nstime::Nanosecond = Nanosecond(0), + id::Union{UUID, Nothing} = nothing, + smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), +) + Base.depwarn( + "`FactorCompute` constructor with `GenericFunctionNodeData` is deprecated. observation, state, and solvercache should be provided explicitly.", + :FactorCompute, + ) + observation = getFactorType(solverData) + state = FactorState( + solverData.eliminated, + solverData.potentialused, + solverData.multihypo, + solverData.certainhypo, + solverData.nullhypo, + solverData.solveInProgress, + solverData.inflation, + ) + + if solverData.fnc isa FactorSolverCache + solvercache = solverData.fnc + else + solvercache = nothing + end + + return FactorCompute( + label, + Tuple(variableOrder), + observation, + state, + solvercache; + id, + timestamp, + nstime, + tags, + smallData, + solvable, + ) +end + +# Deprecated check usefull? # packedFnc = fncStringToData(factor.fnctype, factor.data) +# Deprecated check usefull? # decodeType = getFactorOperationalMemoryType(dfg) +# Deprecated check usefull? # fullFactorData = decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) +function fncStringToData(args...; kwargs...) + @warn "fncStringToData is obsolete, called with" args kwargs + return error("fncStringToData is obsolete.") +end + ## ================================================================================ ## Deprecated in v0.25 ##================================================================================= diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 9f09a598..981cb887 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -248,7 +248,6 @@ export @format_str # Factors ##------------------------------------------------------------------------------ # Factor Data -export GenericFunctionNodeData, PackedFunctionNodeData, FunctionNodeData export AbstractFactorObservation, AbstractPackedFactorObservation export AbstractPrior, AbstractRelative, AbstractRelativeMinimize, AbstractManifoldMinimize export FactorSolverCache @@ -303,7 +302,6 @@ export compare, compareField, compareFields, compareAll, - compareAllSpecial, compareVariable, compareFactor, compareAllVariables, diff --git a/src/FileDFG/services/FileDFG.jl b/src/FileDFG/services/FileDFG.jl index 84a71f33..3eac17cf 100644 --- a/src/FileDFG/services/FileDFG.jl +++ b/src/FileDFG/services/FileDFG.jl @@ -168,7 +168,7 @@ function loadDFG!( jstr = read("$varFolder/$varFile", String) packedvar = JSON3.read(jstr, VariableDFG) v = usePackedVariable ? packedvar : unpackVariable(packedvar) - addVariable!(dfgLoadInto, v) + return addVariable!(dfgLoadInto, v) end @info "Loaded $(length(variables)) variables"#- $(map(v->v.label, variables))" @@ -181,7 +181,7 @@ function loadDFG!( jstr = read("$factorFolder/$factorFile", String) packedfact = JSON3.read(jstr, FactorDFG) f = usePackedFactor ? packedfact : unpackFactor(packedfact) - addFactor!(dfgLoadInto, f) + return addFactor!(dfgLoadInto, f) end @info "Loaded $(length(factors)) factors"# - $(map(f->f.label, factors))" diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index 951de48c..fd8dea8f 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -18,36 +18,6 @@ abstract type FactorSolverCache end # we can add to IIF or have IIF.CommonConvWrapper <: FactorSolverCache directly # abstract type ConvolutionObject <: FactorSolverCache end -##============================================================================== -## GenericFunctionNodeData -##============================================================================== - -""" -$(TYPEDEF) - -Notes -- S::Symbol - -Designing (WIP) -- T <: Union{FactorSolverCache, AbstractPackedFactorObservation} -- in IIF.CCW{T <: DFG.AbstractFactorObservation} -- in DFG.AbstractRelativeMinimize <: AbstractFactorObservation -- in Main.SomeFactor <: AbstractRelativeMinimize -""" -Base.@kwdef mutable struct GenericFunctionNodeData{ - T <: Union{<:AbstractPackedFactorObservation, <:AbstractFactorObservation, <:FactorSolverCache}, -} - eliminated::Bool = false - potentialused::Bool = false - edgeIDs::Vector{Int} = Int[] - fnc::T - multihypo::Vector{Float64} = Float64[] # TODO re-evaluate after refactoring w #477 - certainhypo::Vector{Int} = Int[] - nullhypo::Float64 = 0.0 - solveInProgress::Int = 0 - inflation::Float64 = 0.0 -end - #TODO is this mutable @kwdef mutable struct FactorState eliminated::Bool = false @@ -66,25 +36,6 @@ end ## Constructors -##------------------------------------------------------------------------------ -## PackedFunctionNodeData and FunctionNodeData - -const PackedFunctionNodeData{T} = - GenericFunctionNodeData{T} where {T <: AbstractPackedFactorObservation} -function PackedFunctionNodeData(args...; kw...) - return PackedFunctionNodeData{typeof(args[4])}(args...; kw...) -end - -const FunctionNodeData{T} = GenericFunctionNodeData{ - T, -} where {T <: Union{<:AbstractFactorObservation, <:FactorSolverCache}} -FunctionNodeData(args...; kw...) = FunctionNodeData{typeof(args[4])}(args...; kw...) - -# PackedFunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: AbstractPackedFactorObservation = -# GenericFunctionNodeData{T}(x2, x3, x4, x6, multihypo, certainhypo, x9) -# FunctionNodeData(x2, x3, x4, x6::T, multihypo::Vector{Float64}=[], certainhypo::Vector{Int}=Int[], x9::Int=0) where T <: Union{AbstractFactorObservation, FactorSolverCache} = -# GenericFunctionNodeData{T}(x2, x3, x4, x6, multihypo, certainhypo, x9) - ##============================================================================== ## Factors ##============================================================================== @@ -114,7 +65,7 @@ Base.@kwdef struct FactorDFG <: AbstractDFGFactor nstime::String fnctype::String solvable::Int - data::Union{Nothing, String} #TODO deprecate data completely, left as a bridge to old serialization structure + data::Union{Nothing, String} = nothing #TODO deprecate data completely, left as a bridge to old serialization structure metadata::String _version::String = string(_getDFGVersion()) state::FactorState @@ -129,7 +80,7 @@ end StructTypes.StructType(::Type{FactorDFG}) = StructTypes.UnorderedStruct() StructTypes.idproperty(::Type{FactorDFG}) = :id -StructTypes.omitempties(::Type{FactorDFG}) = (:id,) +StructTypes.omitempties(::Type{FactorDFG}) = (:id, :data) #TODO deprecate, added in v0.27 as a bridge to new serialization structure function FactorDFG( @@ -189,10 +140,6 @@ abstract type InferenceType <: AbstractPackedFactorObservation end #TODO deprecate InferenceType in favor of AbstractPackedFactorObservation v0.26 -# this is the GenericFunctionNodeData for packed types -#TODO deprecate FactorData in favor of FactorState (with no more distinction between packed and compute) -const FactorData = PackedFunctionNodeData{AbstractPackedFactorObservation} - # Packed Factor constructor function assembleFactorName(xisyms::Union{Vector{String}, Vector{Symbol}}) return Symbol(xisyms..., "_f", randstring(4)) @@ -331,57 +278,11 @@ function FactorCompute( ) end -function FactorCompute( - label::Symbol, - variableOrder::Union{Vector{Symbol}, Tuple}, - solverData::GenericFunctionNodeData; - tags::Set{Symbol} = Set{Symbol}(), - timestamp::Union{DateTime, ZonedDateTime} = now(localzone()), - solvable::Int = 1, - nstime::Nanosecond = Nanosecond(0), - id::Union{UUID, Nothing} = nothing, - smallData::Dict{Symbol, SmallDataTypes} = Dict{Symbol, SmallDataTypes}(), -) - Base.depwarn( - "`FactorCompute` constructor with `GenericFunctionNodeData` is deprecated. observation, state, and solvercache should be provided explicitly.", - :FactorCompute, - ) - observation = getFactorType(solverData) - state = FactorState( - solverData.eliminated, - solverData.potentialused, - solverData.multihypo, - solverData.certainhypo, - solverData.nullhypo, - solverData.solveInProgress, - solverData.inflation, - ) - - if solverData.fnc isa FactorSolverCache - solvercache = solverData.fnc - else - solvercache = nothing - end - - return FactorCompute( - label, - Tuple(variableOrder), - observation, - state, - solvercache; - id, - timestamp, - nstime, - tags, - smallData, - solvable, - ) -end - function Base.getproperty(x::FactorCompute, f::Symbol) if f == :solvable getfield(x, f)[] elseif f == :solverData + # TODO remove, deprecated in v0.27 error( "`solverData` is obsolete in `FactorCompute`. Use `getObservation`, `getState` or `getCache` instead.", ) diff --git a/src/services/CompareUtils.jl b/src/services/CompareUtils.jl index 92f02d15..9ca1b6f9 100644 --- a/src/services/CompareUtils.jl +++ b/src/services/CompareUtils.jl @@ -25,7 +25,6 @@ const GeneratedCompareUnion = Union{ VariableDFG, VariableSummary, VariableSkeleton, - GenericFunctionNodeData, FactorCompute, FactorDFG, FactorSummary, @@ -267,38 +266,6 @@ function compareVariable( return TP::Bool end -function compareAllSpecial( - A::T1, - B::T2; - skip = Symbol[], - show::Bool = true, -) where {T1 <: GenericFunctionNodeData, T2 <: GenericFunctionNodeData} - if T1 != T2 - @warn "compareAllSpecial is comparing different types" T1 T2 - # return false - # else - end - return compareAll(A, B; skip = skip, show = show) -end - -# Compare FunctionNodeData -function compare( - a::GenericFunctionNodeData{T1}, - b::GenericFunctionNodeData{T2}, -) where {T1, T2} - # TODO -- beef up this comparison to include the gwp - TP = true - TP = TP && a.eliminated == b.eliminated - TP = TP && a.potentialused == b.potentialused - TP = TP && a.edgeIDs == b.edgeIDs - # TP = TP && typeof(a.fnc) == typeof(b.fnc) - TP = TP && (a.multihypo - b.multihypo |> norm < 1e-10) - TP = TP && a.certainhypo == b.certainhypo - TP = TP && a.nullhypo == b.nullhypo - TP = TP && a.solveInProgress == b.solveInProgress - return TP -end - """ $SIGNATURES @@ -406,7 +373,6 @@ end # Bd = getSolverData(B) # TP = compareAll(A, B, skip=[:attributes;:data], show=show) # TP &= compareAll(A.attributes, B.attributes, skip=[:data;], show=show) -# TP &= compareAllSpecial(getSolverData(A).fnc, getSolverData(B).fnc, skip=[:cpt;], show=show) # TP &= compareAll(getSolverData(A).fnc.cpt, getSolverData(B).fnc.cpt, show=show) """ diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index d4add7cd..85424b1c 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -6,10 +6,6 @@ function getMetadata(f::FactorDFG) return JSON3.read(base64decode(f.metadata), Dict{Symbol, SmallDataTypes}) end -##============================================================================== -## GenericFunctionNodeData -##============================================================================== - ## COMMON # getSolveInProgress # isSolveInProgress @@ -20,7 +16,6 @@ end Return reference to the user factor in `<:AbstractDFG` identified by `::Symbol`. """ -getFactorFunction(fcd::GenericFunctionNodeData) = fcd.fnc.usrfnc! getFactorFunction(fc::FactorCompute) = getObservation(fc) getFactorFunction(dfg::AbstractDFG, fsym::Symbol) = getFactorFunction(getFactor(dfg, fsym)) @@ -32,16 +27,6 @@ Return user factor type from factor graph identified by label `::Symbol`. Notes - Replaces older `getfnctype`. """ -function getFactorType(data::GenericFunctionNodeData{<:FactorSolverCache}) - #TODO deprecated in v0.27 - Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) - return data.fnc.usrfnc! -end -function getFactorType(data::GenericFunctionNodeData{<:AbstractFactorObservation}) - #TODO deprecated in v0.27 - Base.depwarn("getFactorType(::GenericFunctionNodeData) is deprecated", :getFactorType) - return data.fnc -end getFactorType(fct::FactorCompute) = getObservation(fct) getFactorType(f::FactorDFG) = getTypeFromSerializationModule(f.fnctype)() # TODO find a better way to do this that does not rely on empty constructor getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl)) @@ -130,8 +115,8 @@ macro defFactorType(structname, factortype, manifold) ") is not an `AbstractManifold`" @assert ($factortype <: AbstractFactorObservation) "@defFactorType factortype (" * - string($factortype) * - ") is not an `AbstractFactorObservation`" + string($factortype) * + ") is not an `AbstractFactorObservation`" Base.@__doc__ struct $structname{T} <: $factortype Z::T diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index af29cf90..614b46d9 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -270,71 +270,6 @@ end packFactor(f::FactorDFG) = f -function fncStringToData(packtype::Type{<:AbstractPackedFactorObservation}, data::String) - - # Read string as JSON object to use as kwargs - fncData = JSON3.read(if data[1] == '{' - data - else - String(base64decode(data)) - end) - packT = packtype(; fncData.fnc...) - - packed = GenericFunctionNodeData{packtype}( - fncData["eliminated"], - fncData["potentialused"], - fncData["edgeIDs"], - # NamedTuple args become kwargs with the splat - packT, - fncData["multihypo"], - fncData["certainhypo"], - fncData["nullhypo"], - fncData["solveInProgress"], - fncData["inflation"], - ) - return packed -end - -function fncStringToData(packtype::Type{<:AbstractPackedFactorObservation}, data::NamedTuple) - return error( - "Who is calling deserialize factor with NamedTuple, likely JSON3 somewhere", - ) -end - -function fncStringToData( - ::Type{T}, - data::PackedFunctionNodeData{T}, -) where {T <: AbstractPackedFactorObservation} - return data -end -function fncStringToData( - fncType::String, - data::PackedFunctionNodeData{T}, -) where {T <: AbstractPackedFactorObservation} - packtype = DFG.getTypeFromSerializationModule("Packed" * fncType) - if packtype == T - data - else - error( - "Unknown type conversion\n$(fncType)\n$packtype\n$(PackedFunctionNodeData{T})", - ) - end -end - -function fncStringToData(fncType::String, data::T) where {T <: AbstractPackedFactorObservation} - packtype = DFG.getTypeFromSerializationModule("Packed" * fncType) - if packtype == T # || T <: packtype - data - else - fncStringToData(packtype, data) - end -end -function fncStringToData(fncType::String, data::Union{String, <:NamedTuple}) - # FIXME, should rather just store the data as `PackedFactorX` rather than hard code the type change here??? - packtype = DFG.getTypeFromSerializationModule("Packed" * fncType) - return fncStringToData(packtype, data) -end - function unpackObservation(factor::FactorDFG) try return unpack(getObservation(factor)) @@ -375,10 +310,6 @@ function packObservation(observ::AbstractFactorObservation) end end -# Deprecated check usefull? # packedFnc = fncStringToData(factor.fnctype, factor.data) -# Deprecated check usefull? # decodeType = getFactorOperationalMemoryType(dfg) -# Deprecated check usefull? # fullFactorData = decodePackedType(dfg, factor._variableOrderSymbols, decodeType, packedFnc) - function unpackFactor(factor::FactorDFG; skipVersionCheck::Bool = false) # @debug "DECODING factor type = '$(factor.fnctype)' for factor '$(factor.label)'" diff --git a/test/compareTests.jl b/test/compareTests.jl index 44e059b8..7751ac1e 100644 --- a/test/compareTests.jl +++ b/test/compareTests.jl @@ -43,28 +43,23 @@ v2.solvable = 0 VariableCompute(:x1, TestVariableType1()) == VariableCompute(:x1, TestVariableType2()) ) -# GenericFunctionNodeData -gfnd1 = GenericFunctionNodeData(; +facstate1 = DFG.FactorState(; eliminated = true, potentialused = true, - edgeIDs = [1, 2], - fnc = TestFunctorInferenceType1(), ) -gfnd2 = deepcopy(gfnd1) -gfnd3 = GenericFunctionNodeData(; +facstate2 = deepcopy(facstate1) +facstate3 = DFG.FactorState(; eliminated = true, - potentialused = true, - edgeIDs = [1, 2], - fnc = TestFunctorInferenceType2(), + potentialused = false, ) -@test gfnd1 == gfnd2 -@test !(gfnd1 == gfnd3) +@test facstate1 == facstate2 +@test !(facstate1 == facstate3) # FactorCompute -f1 = FactorCompute(:f1, [:a, :b], gfnd1) +f1 = FactorCompute(:f1, [:a, :b], TestFunctorInferenceType1()) f2 = deepcopy(f1) -f3 = FactorCompute(:f1, [:b, :a], gfnd1) +f3 = FactorCompute(:f1, [:b, :a], TestFunctorInferenceType1()) @test f1 == f2 @test !(f1 == f3) @@ -84,19 +79,3 @@ vnd2.val[1][1] = 0.1 @test !compare(vnd1, vnd2) @test !compare(vnd1, vnd3) -gfnd1 = GenericFunctionNodeData(; - eliminated = true, - potentialused = true, - edgeIDs = [1, 2], - fnc = TestFunctorInferenceType1(), -) -gfnd2 = deepcopy(gfnd1) -gfnd3 = GenericFunctionNodeData(; - eliminated = true, - potentialused = true, - edgeIDs = [1, 2], - fnc = PackedTestFunctorInferenceType1(), -) - -@test compare(gfnd1, gfnd2) -@test_broken !(compare(gfnd1, gfnd3)) diff --git a/test/fileDFGTests.jl b/test/fileDFGTests.jl index b8ec87ff..09e8e349 100644 --- a/test/fileDFGTests.jl +++ b/test/fileDFGTests.jl @@ -158,7 +158,10 @@ using UUIDs end ## - +# file = filter(f -> endswith(f, ".tar.gz"), readdir(joinpath(@__DIR__, "data")))[2] +# loadFile = joinpath(@__DIR__, "data", file) +# fg = loadDFG(loadFile) +## @testset "FileDFG Regression Tests" begin @info "If any of these tests fail, we have breaking changes" for file in filter(f -> endswith(f, ".tar.gz"), readdir(joinpath(@__DIR__, "data"))) diff --git a/test/plottingTest.jl b/test/plottingTest.jl index 2ec2159c..764de2eb 100644 --- a/test/plottingTest.jl +++ b/test/plottingTest.jl @@ -20,7 +20,7 @@ map( FactorCompute( Symbol("x$(n)x$(n+1)f1"), [verts[n].label, verts[n + 1].label], - GenericFunctionNodeData(; fnc=TestFunctorInferenceType1()) + TestFunctorInferenceType1() ), ), 1:(numNodes - 1), diff --git a/test/runtests.jl b/test/runtests.jl index b9cdcb01..ff8bf69f 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -62,7 +62,9 @@ if get(ENV, "IIF_TEST", "true") == "true" # Switch to our upstream test branch. Pkg.add( - PackageSpec(; name = "IncrementalInference", rev = "upstream/dfg_integration_test"), + #FIXME This is a temporary fix to use the refactored factor branch. + # PackageSpec(; name = "IncrementalInference", rev = "upstream/dfg_integration_test"), + PackageSpec(; name = "IncrementalInference", rev = "refac/factor"), ) @info "------------------------------------------------------------------------" @info "These tests are using IncrementalInference to do additional driver tests" diff --git a/test/testBlocks.jl b/test/testBlocks.jl index ebe4c6de..4b2e7cf8 100644 --- a/test/testBlocks.jl +++ b/test/testBlocks.jl @@ -392,16 +392,16 @@ function DFGFactorSCA() f1_tags = Set([:FACTOR]) testTimestamp = now(localzone()) - gfnd_prior = GenericFunctionNodeData(; fnc = TestCCW(TestAbstractPrior())) + obs_prior = TestAbstractPrior() - gfnd = GenericFunctionNodeData(; fnc = TestCCW(TestFunctorInferenceType1())) + obs = TestFunctorInferenceType1() - f1 = FactorCompute(f1_lbl, [:a, :b], gfnd; tags = f1_tags, solvable = 0) + f1 = FactorCompute(f1_lbl, [:a, :b], obs; tags = f1_tags, solvable = 0) f2 = FactorCompute( :bcf1, [:b, :c], - GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}()); + TestFunctorInferenceType1(); timestamp = ZonedDateTime("2020-08-11T00:12:03.000-05:00"), ) #TODO add tests for mutating vos in updateFactor and orphan related checks. @@ -445,7 +445,7 @@ function DFGFactorSCA() @test getSolvable(f1) == 1 # create f0 here for a later timestamp - f0 = FactorCompute(:af1, [:a], gfnd_prior; tags = Set([:PRIOR])) + f0 = FactorCompute(:af1, [:a], obs_prior; tags = Set([:PRIOR])) #fill in undefined fields # f2.solverData.certainhypo = Int[] @@ -471,7 +471,7 @@ function VariablesandFactorsCRUD_SET!(fg, v1, v2, v3, f0, f1, f2) @test getLabel(fg[getLabel(v1)]) == getLabel(v1) #TODO standardize this error and res also for that matter - fnope = FactorCompute(:broken, [:a, :nope], GenericFunctionNodeData(; fnc=TestCCW{TestFunctorInferenceType1}())) + fnope = FactorCompute(:broken, [:a, :nope], TestFunctorInferenceType1()) @test_throws KeyError addFactor!(fg, fnope) @test addFactor!(fg, f1) == f1 @@ -1375,12 +1375,6 @@ function testGroup!(fg, v1, v2, f0, f1) end end -# Feed with graph a b solvable orphan not factor on a b -# fg = testDFGAPI() -# addVariable!(fg, VariableCompute(:a, TestVariableType1())) -# addVariable!(fg, VariableCompute(:b, TestVariableType1())) -# addFactor!(fg, FactorCompute(:abf1, [:a,:b], GenericFunctionNodeData{TestFunctorInferenceType1, Symbol}())) -# addVariable!(fg, VariableCompute(:orphan, TestVariableType1(), solvable = 0)) function AdjacencyMatricesTestBlock(fg) # Normal #deprecated @@ -1452,17 +1446,15 @@ function connectivityTestGraph( setSolvable!(dfg, :x8, 0) setSolvable!(dfg, :x9, 0) - gfnd = GenericFunctionNodeData(; + facstate = DFG.FactorState(; eliminated = true, potentialused = true, - fnc = TestCCW(TestFunctorInferenceType1()), multihypo = Float64[], certainhypo = Int[], solveInProgress = 0, inflation = 1.0, ) f_tags = Set([:FACTOR]) - # f1 = FactorCompute(f1_lbl, [:a,:b], gfnd, tags = f_tags) facs = map( n -> addFactor!( @@ -1470,7 +1462,8 @@ function connectivityTestGraph( FactorCompute( Symbol("x$(n)x$(n+1)f1"), [vars[n].label, vars[n + 1].label], - deepcopy(gfnd); + TestFunctorInferenceType1(), + facstate; tags = deepcopy(f_tags), ), ), @@ -1681,7 +1674,7 @@ function ProducingDotFiles( end if f1 === nothing if (FACTYPE == FactorCompute) - f1 = FactorCompute(:abf1, [:a, :b], GenericFunctionNodeData(;fnc = TestFunctorInferenceType1())) + f1 = FactorCompute(:abf1, [:a, :b], TestFunctorInferenceType1()) else f1 = FACTYPE(:abf1, [:a, :b]) end From 55f14d30264567e8d0d73f92f11b25770aceb333 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Tue, 17 Jun 2025 22:42:54 +0200 Subject: [PATCH 11/14] fix broken docs --- src/Deprecated.jl | 12 ------------ src/DistributedFactorGraphs.jl | 2 +- src/entities/DFGFactor.jl | 5 ++--- src/services/DFGFactor.jl | 26 ++++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/Deprecated.jl b/src/Deprecated.jl index 2d1b5d04..d7cda9b8 100644 --- a/src/Deprecated.jl +++ b/src/Deprecated.jl @@ -203,18 +203,6 @@ function updateVariableSolverData!( end ## factor refactor deprecations -""" -$(TYPEDEF) - -Notes -- S::Symbol - -Designing (WIP) -- T <: Union{FactorSolverCache, AbstractPackedFactorObservation} -- in IIF.CCW{T <: DFG.AbstractFactorObservation} -- in DFG.AbstractRelativeMinimize <: AbstractFactorObservation -- in Main.SomeFactor <: AbstractRelativeMinimize -""" Base.@kwdef mutable struct GenericFunctionNodeData{ T <: Union{ <:AbstractPackedFactorObservation, diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 981cb887..92cae9bb 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -43,7 +43,7 @@ using Tables # used for @defVariable import ManifoldsBase -import ManifoldsBase: AbstractManifold, manifold_dimension +using ManifoldsBase: AbstractManifold, manifold_dimension export AbstractManifold, manifold_dimension import RecursiveArrayTools: ArrayPartition diff --git a/src/entities/DFGFactor.jl b/src/entities/DFGFactor.jl index fd8dea8f..6a7e3970 100644 --- a/src/entities/DFGFactor.jl +++ b/src/entities/DFGFactor.jl @@ -48,14 +48,13 @@ end # | FactorCompute | X | X | X | X | X | # *not available without reconstruction +#TODO is packed observation a abstract type, parameter, or is it already a string? +#TODO Same with metadata? """ $(TYPEDEF) The Factor information packed in a way that accomdates multi-lang using json. """ - -#TODO do we have parameter for packed observation or is it already a string? -#TODO Same with metadata? Base.@kwdef struct FactorDFG <: AbstractDFGFactor id::Union{UUID, Nothing} = nothing label::Symbol diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 85424b1c..31847a64 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -33,9 +33,20 @@ getFactorType(dfg::AbstractDFG, lbl::Symbol) = getFactorType(getFactor(dfg, lbl) getState(f::AbstractDFGFactor) = f.state +""" + $SIGNATURES + +Return factor state from factor graph. +""" getFactorState(f::AbstractDFGFactor) = f.state getFactorState(dfg::AbstractDFG, lbl::Symbol) = getFactorState(getFactor(dfg, lbl)) +""" + $SIGNATURES + +Return the observation of a factor, which is the user-defined data structure +that contains the information about the factor, such as the measurement, prior, or relative pose. +""" getObservation(f::FactorCompute) = f.observation function getObservation(f::FactorDFG) #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes @@ -46,6 +57,13 @@ end getObservation(dfg::AbstractDFG, lbl::Symbol) = getObservation(getFactor(dfg, lbl)) +""" + $SIGNATURES + +Return the solver cache for a factor, which is used to store intermediate results +during the solving process. This is useful for caching results that can be reused +across multiple solves, such as Jacobians or other computed values. +""" function getCache(f::FactorCompute) if isassigned(f.solvercache) return f.solvercache[] @@ -53,6 +71,14 @@ function getCache(f::FactorCompute) return nothing end end + +""" + $SIGNATURES + +Set the solver cache for a factor, which is used to store intermediate results +during the solving process. This is useful for caching results that can be reused +across multiple solves, such as Jacobians or other computed values. +""" setCache!(f::FactorCompute, solvercache::FactorSolverCache) = f.solvercache[] = solvercache """ From fa2843517f503dabe54904d0d6aaeffb72188159 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Wed, 18 Jun 2025 14:55:40 +0200 Subject: [PATCH 12/14] fix defFactorType -> getManifold --- src/DistributedFactorGraphs.jl | 2 +- src/services/DFGFactor.jl | 4 +++- src/services/Serialization.jl | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl index 92cae9bb..8cdee465 100644 --- a/src/DistributedFactorGraphs.jl +++ b/src/DistributedFactorGraphs.jl @@ -167,7 +167,7 @@ export getSolverDataDict, setSolverData! export getVariableType, getVariableTypeName export getObservation -export getState +export getState, getFactorState export getVariableType diff --git a/src/services/DFGFactor.jl b/src/services/DFGFactor.jl index 31847a64..8120450c 100644 --- a/src/services/DFGFactor.jl +++ b/src/services/DFGFactor.jl @@ -155,13 +155,15 @@ macro defFactorType(structname, factortype, manifold) # $structname(; Z) = $structname(Z) $packedstructname(; Z) = $packedstructname(Z) - DFG.getManifold(::Type{$structname}) = $manifold + DFG.getManifold(::Type{<:$structname}) = $manifold DFG.pack(d::$structname) = $packedstructname(DFG.packDistribution(d.Z)) DFG.unpack(d::$packedstructname) = $structname(DFG.unpackDistribution(d.Z)) end, ) end +getManifold(obs::AbstractFactorObservation) = getManifold(typeof(obs)) + ##============================================================================== ## Factors ##============================================================================== diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl index 614b46d9..65e07d50 100644 --- a/src/services/Serialization.jl +++ b/src/services/Serialization.jl @@ -282,6 +282,7 @@ function unpackObservation(factor::FactorDFG) ) #FIXME completely refactor to not need getTypeFromSerializationModule and just use StructTypes #TODO change to unpack: observ = unpack(observpacked) + # currently the observation type is stored in the factor and this complicates unpacking of seperate observations observpacked = getObservation(factor) packtype = DFG.getTypeFromSerializationModule("Packed" * factor.fnctype) return convert(convertStructType(packtype), observpacked) From 48976ef44dfa7e25036ff9b5202636e2e45f1dd3 Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Wed, 18 Jun 2025 15:46:46 +0200 Subject: [PATCH 13/14] update news and bump to v0.27 --- NEWS.md | 6 ++++++ Project.toml | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index ce999a40..2e2388f8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,12 @@ Listing news on any major breaking changes in DFG. For regular changes, see int - Deprecate `updateFactor!` for `mergeFactor!`, note `merege` returns number of nodes updated/added. - Rename BlobEntry to Blobentry, see #1123. - Rename BlobStore to Blobstore, see #1124. +- Refactor the Factor solver data structure, see #1127: + - Deprecated GenericFunctionNodeData, PackedFunctionNodeData, FunctionNodeData, and all functions related factor.solverData. + - Replaced by 3 seperete types: Observation, State, and Cache + - This is used internally be the solver and should not affect the average user of DFG. + - Rename FactorOperationalMemory -> FactorSolverCache + - Rename AbstractFactor -> AbstractFactorObservation (keeping both around) # v0.26 - Graph structure plotting now uses GraphMakie.jl instead of GraphPlot.jl. Update by replacing `using GraphPlot` with `using GraphMakie`. diff --git a/Project.toml b/Project.toml index 19b12273..5ab17ac9 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "DistributedFactorGraphs" uuid = "b5cc3c7e-6572-11e9-2517-99fb8daf2f04" -version = "0.26.0" +version = "0.27.0" [deps] Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" From 4e285b45c91081024eb0d3511248f502726142ad Mon Sep 17 00:00:00 2001 From: Johannes Terblanche Date: Wed, 18 Jun 2025 15:54:41 +0200 Subject: [PATCH 14/14] fix formatting --- test/compareTests.jl | 11 ++--------- test/fileDFGTests.jl | 2 +- test/plottingTest.jl | 2 +- test/runtests.jl | 5 +---- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/test/compareTests.jl b/test/compareTests.jl index 7751ac1e..a1ebf7d6 100644 --- a/test/compareTests.jl +++ b/test/compareTests.jl @@ -43,15 +43,9 @@ v2.solvable = 0 VariableCompute(:x1, TestVariableType1()) == VariableCompute(:x1, TestVariableType2()) ) -facstate1 = DFG.FactorState(; - eliminated = true, - potentialused = true, -) +facstate1 = DFG.FactorState(; eliminated = true, potentialused = true) facstate2 = deepcopy(facstate1) -facstate3 = DFG.FactorState(; - eliminated = true, - potentialused = false, -) +facstate3 = DFG.FactorState(; eliminated = true, potentialused = false) @test facstate1 == facstate2 @test !(facstate1 == facstate3) @@ -78,4 +72,3 @@ push!(vnd2.val, [1.0;]) vnd2.val[1][1] = 0.1 @test !compare(vnd1, vnd2) @test !compare(vnd1, vnd3) - diff --git a/test/fileDFGTests.jl b/test/fileDFGTests.jl index 09e8e349..7ecef346 100644 --- a/test/fileDFGTests.jl +++ b/test/fileDFGTests.jl @@ -135,7 +135,7 @@ using UUIDs @test compareFactor( getFactor(dfg, fact), getFactor(retDFG, fact), - skip = [:timezone, :zone, :solverData,], + skip = [:timezone, :zone, :solverData], ) # Timezones # :hypotheses, :certainhypo, :multihypo, # Multihypo # :eliminated, diff --git a/test/plottingTest.jl b/test/plottingTest.jl index 764de2eb..220f9c06 100644 --- a/test/plottingTest.jl +++ b/test/plottingTest.jl @@ -20,7 +20,7 @@ map( FactorCompute( Symbol("x$(n)x$(n+1)f1"), [verts[n].label, verts[n + 1].label], - TestFunctorInferenceType1() + TestFunctorInferenceType1(), ), ), 1:(numNodes - 1), diff --git a/test/runtests.jl b/test/runtests.jl index ff8bf69f..e354e9fd 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -73,10 +73,7 @@ if get(ENV, "IIF_TEST", "true") == "true" using IncrementalInference apis = Vector{AbstractDFG}() - push!( - apis, - GraphsDFG(; solverParams = SolverParams()), - ) + push!(apis, GraphsDFG(; solverParams = SolverParams())) for api in apis @testset "Testing Driver: $(typeof(api))" begin