Merge pull request #949 from JuliaRobotics/23Q1/fix/tryquickkwfix

Fix constructor CI bug, JL1.9, drop Unmarshal, and more

Author: dehann

Project.toml

@@ -1,6 +1,6 @@
 name = "DistributedFactorGraphs"
 uuid = "b5cc3c7e-6572-11e9-2517-99fb8daf2f04"
-version = "0.19.2"
+version = "0.19.3"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"
@@ -25,7 +25,6 @@ SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
 TensorCast = "02d47bb6-7ce6-556a-be16-bb1710789e2b"
 TimeZones = "f269a46b-ccf7-5d73-abea-4c690281aa53"
 UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
-Unmarshal = "cbff2730-442d-58d7-89d1-8e530c41eb02"
 
 [compat]
 Colors = "0.10, 0.11, 0.12"
@@ -45,7 +44,6 @@ Reexport = "1"
 Requires = "1"
 TensorCast = "0.3.3, 0.4"
 TimeZones = "1.3.1"
-Unmarshal = "0.4"
 julia = "1.6"
 
 [extras]

src/DataBlobs/entities/AbstractDataEntries.jl

@@ -28,38 +28,39 @@ General Data Store Entry.
     origin::String # E.g. user|robot|session|varlabel
     description::String
     mimeType::String
-    metadata::String
+    metadata::String = ""
     timestamp::ZonedDateTime = now(localzone())
     _type::String = "BlobStoreEntry"
     _version::String = _getDFGVersion()
 end
 
 _fixtimezone(cts::NamedTuple) = ZonedDateTime(cts.utc_datetime*"+00")
 
-# needed for deserialization from JSON during DFG v0.19 transition, see #867
-function BlobStoreEntry(;
-    id,
-    label,
-    blobstore,
-    hash,
-    origin,
-    description,
-    mimeType,
-    timestamp,
-    kwargs... # drop excessive fields
-)
-    #
-    BlobStoreEntry(
-        UUID(id),
-        Symbol(label),
-        Symbol(blobstore),
-        hash,
-        origin,
-        description,
-        mimeType,
-        _fixtimezone(timestamp),
-    )
-end
+# # needed for deserialization from JSON during DFG v0.19 transition, see #867
+# # TODO this function can likely be removed, since julia automatically tries type conversion on constructors.
+# function BlobStoreEntry(;
+#     id,
+#     label,
+#     blobstore,
+#     hash,
+#     origin,
+#     description,
+#     mimeType,
+#     timestamp,
+#     kwargs... # drop excessive fields
+# )
+#     #
+#     BlobStoreEntry(;
+#         id=UUID(id),
+#         label=Symbol(label),
+#         blobstore=Symbol(blobstore),
+#         hash,
+#         origin,
+#         description,
+#         mimeType,
+#         timestamp=_fixtimezone(timestamp),
+#     )
+# end
 
 # TODO
 """

src/Deprecated.jl

@@ -63,6 +63,26 @@ end
 ## Deprecate before v0.20
 ##=================================================================================
 
+
+# Need to implement this to allow Unmarshal to deserialize Nullable UUIDs and ZonedDateTimes
+# TODO: Move to JSON3.
+# import Unmarshal: unmarshal
+
+function unmarshal(::Type{Union{UUID, Nothing}}, x, verbose :: Bool = false, verboseLvl :: Int = 0)
+    if x !== nothing 
+        return UUID(x)
+    else 
+        return nothing
+    end
+end
+function unmarshal(::Type{Union{ZonedDateTime, Nothing}}, x, verbose :: Bool = false, verboseLvl :: Int = 0) 
+    if x !== nothing 
+        return ZonedDateTime(x)
+    else 
+        return nothing
+    end
+end
+
 export DefaultDFG
 
 const DefaultDFG = GraphsDFG

src/DistributedFactorGraphs.jl

@@ -23,7 +23,7 @@ using TimeZones
 using Distributions
 using Reexport
 using JSON
-using Unmarshal
+# using Unmarshal
 using JSON2 # JSON2 requires all properties to be in correct sequence, can't guarantee that from DB.
 using JSON3
 using LinearAlgebra

src/GraphsDFG/entities/GraphsDFG.jl

@@ -29,21 +29,40 @@ Create an in-memory GraphsDFG with the following parameters:
 - V: Variable type
 - F: Factor type
 """
-function GraphsDFG{T,V,F}(g::FactorGraph{Int,V,F}=FactorGraph{Int,V,F}();
-                           description::String="Graphs.jl implementation",
-                           userLabel::String="DefaultUser",
-                           robotLabel::String="DefaultRobot",
-                           sessionLabel::String="Session_$(string(uuid4())[1:6])",
-                           userData::Dict{Symbol, String} = Dict{Symbol, String}(),
-                           robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
-                           sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
-                           solverParams::T=T(),
-                           blobstores=Dict{Symbol, AbstractBlobStore}()) where {T <: AbstractParams, V <:AbstractDFGVariable, F<:AbstractDFGFactor}
-   # Validate the userLabel, robotLabel, and sessionLabel
-   !isValidLabel(userLabel) && error("'$userLabel' is not a valid User label")
-   !isValidLabel(robotLabel) && error("'$robotLabel' is not a valid Robot label")
-   !isValidLabel(sessionLabel) && error("'$sessionLabel' is not a valid Session label")
-   return GraphsDFG{T,V,F}(g, description, userLabel, robotLabel, sessionLabel, userData, robotData, sessionData, Symbol[], solverParams, blobstores)
+function GraphsDFG{T,V,F}(
+    g::FactorGraph{Int,V,F}=FactorGraph{Int,V,F}();
+    description::String="Graphs.jl implementation",
+    userLabel::String="DefaultUser",
+    robotLabel::String="DefaultRobot",
+    sessionLabel::String="Session_$(string(uuid4())[1:6])",
+    userData::Dict{Symbol, String} = Dict{Symbol, String}(),
+    robotData::Dict{Symbol, String} = Dict{Symbol, String}(),
+    sessionData::Dict{Symbol, String} = Dict{Symbol, String}(),
+    solverParams::T=T(),
+    blobstores=Dict{Symbol, AbstractBlobStore}(),
+    # deprecating
+    userId::Union{Nothing, String} = nothing,
+    robotId::Union{Nothing, String} = nothing,
+    sessionId::Union{Nothing, String} = nothing,
+) where {T <: AbstractParams, V <:AbstractDFGVariable, F<:AbstractDFGFactor}
+    # Validate the userLabel, robotLabel, and sessionLabel
+    if userId !== nothing
+        @error "Obsolete use of userId::String with GraphsDFG, use userLabel::String instead" maxlog=10
+        userLabel = userId
+    end
+    if robotId !== nothing
+        @error "Obsolete use of robotId::String with GraphsDFG, use robotLabel::String instead" maxlog=10
+        robotLabel = robotId
+    end
+    if sessionId !== nothing
+        @error "Obsolete use of sessionId::String with GraphsDFG, use sessionLabel::String instead" maxlog=10
+        sessionLabel = sessionId
+    end
+
+    !isValidLabel(userLabel) && error("'$userLabel' is not a valid User label")
+    !isValidLabel(robotLabel) && error("'$robotLabel' is not a valid Robot label")
+    !isValidLabel(sessionLabel) && error("'$sessionLabel' is not a valid Session label")
+    return GraphsDFG{T,V,F}(g, description, userLabel, robotLabel, sessionLabel, userData, robotData, sessionData, Symbol[], solverParams, blobstores)
 end
 
 # GraphsDFG{T}(; kwargs...) where T <: AbstractParams = GraphsDFG{T,DFGVariable,DFGFactor}(;kwargs...)

src/entities/DFGVariable.jl

@@ -17,32 +17,63 @@ Fields:
 $(TYPEDFIELDS)
 """
 Base.@kwdef mutable struct VariableNodeData{T<:InferenceVariable, P}
+    """
+    Globally unique identifier.
+    """
     id::Union{UUID, Nothing} = nothing # If it's blank it doesn't exist in the DB.
+    """
+    Vector of on-manifold points used to represent a ManifoldKernelDensity (or parametric) belief.
+    """
     val::Vector{P}
+    """
+    Common kernel bandwith parameter used with ManifoldKernelDensity, and as legacy also stores covariance until a dedicated field is created for parametric case.
+    """
     bw::Matrix{Float64} = zeros(0,0)
     BayesNetOutVertIDs::Vector{Symbol} = Symbol[]
-    dimIDs::Vector{Int} = Int[] # Likely deprecate
+    dimIDs::Vector{Int} = Int[] # TODO Likely deprecate
 
     dims::Int = 0
+    """
+    Flag used by junction (Bayes) tree construction algorith to know whether this variable has yet been included in the tree construction.
+    """
     eliminated::Bool = false
     BayesNetVertID::Symbol = :NOTHING #  Union{Nothing, }
     separator::Vector{Symbol} = Symbol[]
-
+    """
+    Variables each have a type, such as Position1, or RoME.Pose2, etc.
+    """
     variableType::T
+    """
+    False if initial numerical values are not yet available or stored values are not ready for further processing yet.
+    """
     initialized::Bool = false
     """
-    Replacing previous `inferdim::Float64`, new `.infoPerCoord::Vector{Float64}` will in 
-    future stores the amount information (per measurement dimension) captured in each 
-    coordinate dimension.
+    Stores the amount information (per measurement dimension) captured in each coordinate dimension.
     """
     infoPerCoord::Vector{Float64} = Float64[0.0;]
+    """
+    Should this variable solveKey be treated as marginalized in inference computations.
+    """
     ismargin::Bool = false
-
+    """
+    Shoudl this variable solveKey always be kept fluid and not be automatically marginalized.
+    """
     dontmargin::Bool = false
+    """
+    Convenience flag on whether a solver is currently busy working on this variable solveKey.
+    """
     solveInProgress::Int = 0
+    """
+    How many times has a solver updated this variable solveKey estimte.
+    """
     solvedCount::Int = 0
+    """
+    solveKey identifier associated with thsi VariableNodeData object.
+    """
     solveKey::Symbol
-
+    """
+    Future proofing field for when more multithreading operations on graph nodes are implemented, these conditions are meant to be used for atomic write transactions to this VND.
+    """
     events::Dict{Symbol,Threads.Condition} = Dict{Symbol,Threads.Condition}()
     #
 end
@@ -62,10 +93,6 @@ function VariableNodeData(variableType::T; solveKey::Symbol=:default) where T <:
     bw = zeros(0,0)
     # bw[1] = zeros(getDimension(T))
     VariableNodeData(;  val, bw, solveKey, variableType  )
-    # VariableNodeData(   nothing, P0, BW, Symbol[], Int[], 
-    #                     0, false, :NOTHING, Symbol[], 
-    #                     variableType, false, [0.0], false, 
-    #                     false, 0, 0, solveKey  )
 end
 
 

src/services/Serialization.jl

@@ -12,26 +12,9 @@ import JSON.Writer: StructuralContext, JSONContext, show_json
 import JSON.Serializations: CommonSerialization, StandardSerialization
 JSON.show_json(io::JSONContext, serialization::CommonSerialization, uuid::UUID) = print(io.io, "\"$uuid\"")
 
-# Need to implement this to allow Unmarshal to deserialize Nullable UUIDs and ZonedDateTimes
-# TODO: Move to JSON3.
-import Unmarshal: unmarshal
-
-function unmarshal(::Type{Union{UUID, Nothing}}, x, verbose :: Bool = false, verboseLvl :: Int = 0)
-    if x !== nothing 
-        return UUID(x)
-    else 
-        return nothing
-    end
-end
-function unmarshal(::Type{Union{ZonedDateTime, Nothing}}, x, verbose :: Bool = false, verboseLvl :: Int = 0) 
-    if x !== nothing 
-        return ZonedDateTime(x)
-    else 
-        return nothing
-    end
-end
 
 ## Version checking
+# FIXME return VersionNumber
 function _getDFGVersion()
     if haskey(Pkg.dependencies(), Base.UUID("b5cc3c7e-6572-11e9-2517-99fb8daf2f04"))
         return string(Pkg.dependencies()[Base.UUID("b5cc3c7e-6572-11e9-2517-99fb8daf2f04")].version)
@@ -81,7 +64,7 @@ end
 function standardizeZDTStrings!(T, interm::Dict)
 
     for (name, typ) in zip(fieldnames(T), T.types)
-        if typ <: ZonedDateTime
+        if typ <: ZonedDateTime && haskey(interm, name)
             namestr = string(name)
             interm[namestr] = getStandardZDTString(interm[namestr])
         end
@@ -144,6 +127,14 @@ end
     $SIGNATURES
 Should be a highly reusable function for any transcoding of intermediate type (or dict) to a desired output type.
 
+Notes:
+- Using Base.@kwdef and JSON3.jl probably has better conversion logic than this function.
+- This function was written to reduce dependency on Unmarshal.jl which was becoming stale.
+
+DevNotes
+- See if this function just be deprecated to use JSON3 or similar.
+- Do better with Union{Nothing, T} types (if this function is not replaced by JSON3)
+
 examples
 ```julia
 Base.@kwdef struct HardType
@@ -200,6 +191,7 @@ function transcodeType(
     # specializations as inner functions (don't have to be inners)
     # these few special cases came up with examples below, note recursions
     _instance(S::Type, x) = S(x)
+    _instance(S::Type{Union{Nothing, UUID}}, x::String) = UUID(x) # special case
     _instance(_::Type{S}, x::S) where S = x # if ambiguous, delete and do alternative `_instance(S::Type, x) = S===Any ? x : S(x)`
     _instance(S::Type{I}, x::AbstractString) where I <: Number = Base.parse(I, x)
     _instance(S::Type{E}, x::AbstractVector) where E <: AbstractVector = _instance.(eltype(E),x)
@@ -452,7 +444,8 @@ function unpackVariable(
             interm = _doparse(bdeInter) # JSON.parse(bdeInter) # bdeInter
             objType = getfield(DistributedFactorGraphs, Symbol(dataElemTypes[k]))
             standardizeZDTStrings!(objType, interm)
-            fullVal = Unmarshal.unmarshal(objType, interm)
+            fullVal = transcodeType(objType, interm)
+            # fullVal = Unmarshal.unmarshal(objType, interm)
             variable.dataDict[k] = fullVal
         end
     end