Intermediate removing factor solverData

Author: Affie

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 &&

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

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 & (

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

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,

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, 

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