Merge pull request #449 from JuliaRobotics/maint/20Q2/rebuildFactorMetadata#425

Affie · web-flow · commit f88fc367e9d8 · 2020-05-07T18:38:36.000+02:00
Also dispach on solverData type for rebuildFactorMetadata!
diff --git a/attic/GraphsDFG/entities/GraphsDFG.jl b/attic/GraphsDFG/entities/GraphsDFG.jl
@@ -8,7 +8,7 @@ mutable struct GraphsNode
 end
 const FGType = Graphs.GenericIncidenceList{GraphsNode,Graphs.Edge{GraphsNode},Dict{Int,GraphsNode},Dict{Int,Array{Graphs.Edge{GraphsNode},1}}}
 
-mutable struct GraphsDFG{T <: AbstractParams} <: AbstractDFG
+mutable struct GraphsDFG{T <: AbstractParams} <: AbstractDFG{T}
     g::FGType
     description::String
     userId::String
diff --git a/src/CloudGraphsDFG/entities/CloudGraphsDFG.jl b/src/CloudGraphsDFG/entities/CloudGraphsDFG.jl
@@ -5,16 +5,12 @@ mutable struct Neo4jInstance
   graph::Neo4j.Graph
 end
 
-mutable struct CloudGraphsDFG{T <: AbstractParams} <: AbstractDFG
+mutable struct CloudGraphsDFG{T <: AbstractParams} <: AbstractDFG{T}
     neo4jInstance::Neo4jInstance
     userId::String
     robotId::String
     sessionId::String
     description::String
-    encodePackedTypeFunc #TODO Is this planed to be used or deprecated?
-    getPackedTypeFunc #TODO Is this planed to be used or deprecated?
-    decodePackedTypeFunc #TODO Is this planed to be used or deprecated?
-    rebuildFactorMetadata!
     addHistory::Vector{Symbol}
     solverParams::T # Solver parameters
 end
@@ -27,11 +23,7 @@ function CloudGraphsDFG{T}(neo4jConnection::Neo4j.Connection,
                             userId::String,
                             robotId::String,
                             sessionId::String,
-                            description::String,
-                            encodePackedTypeFunc,
-                            getPackedTypeFunc,
-                            decodePackedTypeFunc,
-                            rebuildFactorMetadata!;
+                            description::String;
                             solverParams::T=NoSolverParams(),
                             createSessionNodes::Bool=true) where T <: AbstractParams
     # Validate the userId, robotId, and sessionId
@@ -41,7 +33,7 @@ function CloudGraphsDFG{T}(neo4jConnection::Neo4j.Connection,
 
     graph = Neo4j.getgraph(neo4jConnection)
     neo4jInstance = Neo4jInstance(neo4jConnection, graph)
-    dfg = CloudGraphsDFG{T}(neo4jInstance, userId, robotId, sessionId, description, encodePackedTypeFunc, getPackedTypeFunc, decodePackedTypeFunc, rebuildFactorMetadata!, Symbol[], solverParams)
+    dfg = CloudGraphsDFG{T}(neo4jInstance, userId, robotId, sessionId, description, Symbol[], solverParams)
     # Create the session if it doesn't already exist
     createSessionNodes && createDfgSessionIfNotExist(dfg)
     return dfg
@@ -57,15 +49,11 @@ function CloudGraphsDFG{T}(host::String,
                            userId::String,
                            robotId::String,
                            sessionId::String,
-                           description::String,
-                           encodePackedTypeFunc,
-                           getPackedTypeFunc,
-                           decodePackedTypeFunc,
-                           rebuildFactorMetadata!;
+                           description::String;
                            solverParams::T=NoSolverParams(),
                            createSessionNodes::Bool=true) where T <: AbstractParams
     neo4jConnection = Neo4j.Connection(host, port=port, user=dbUser, password=dbPassword);
-    return CloudGraphsDFG{T}(neo4jConnection, userId, robotId, sessionId, description, encodePackedTypeFunc, getPackedTypeFunc, decodePackedTypeFunc, rebuildFactorMetadata!, solverParams=solverParams, createSessionNodes=createSessionNodes)
+    return CloudGraphsDFG{T}(neo4jConnection, userId, robotId, sessionId, description, solverParams=solverParams, createSessionNodes=createSessionNodes)
 end
 
 
diff --git a/src/CloudGraphsDFG/services/CloudGraphsDFG.jl b/src/CloudGraphsDFG/services/CloudGraphsDFG.jl
@@ -18,10 +18,6 @@ function _getDuplicatedEmptyDFG(dfg::CloudGraphsDFG)::CloudGraphsDFG
         dfg.robotId,
         sessionId,
         dfg.description,
-        dfg.encodePackedTypeFunc,
-        dfg.getPackedTypeFunc,
-        dfg.decodePackedTypeFunc,
-        dfg.rebuildFactorMetadata!,
         solverParams=deepcopy(dfg.solverParams))
 end
 
@@ -242,7 +238,7 @@ function getFactor(dfg::CloudGraphsDFG, label::Union{Symbol, String})::DFGFactor
     factor = unpackFactor(dfg, props)
 
     # Lastly, rebuild the metadata
-    factor = dfg.rebuildFactorMetadata!(dfg, factor)
+    factor = rebuildFactorMetadata!(dfg, factor)
 
     return factor
 end
diff --git a/src/Deprecated.jl b/src/Deprecated.jl
@@ -11,6 +11,8 @@ include("../attic/GraphsDFG/GraphsDFG.jl")
 @reexport using .GraphsDFGs
 
 
+@deprecate loadDFG(source::String, iifModule::Module, dest::AbstractDFG) loadDFG!(dest, source)
+
 @deprecate buildSubgraphFromLabels!(dfg::AbstractDFG, varList::Vector{Symbol}) buildSubgraph(dfg, varList, 1)
 
 
diff --git a/src/DistributedFactorGraphs.jl b/src/DistributedFactorGraphs.jl
@@ -212,7 +212,7 @@ export setSerializationModule!, getSerializationModule
 export packVariable, unpackVariable, packFactor, unpackFactor
 
 # File import and export
-export saveDFG, loadDFG
+export saveDFG, loadDFG!
 export toDot, toDotFile
 
 # Comparisons
diff --git a/src/FileDFG/services/FileDFG.jl b/src/FileDFG/services/FileDFG.jl
@@ -82,9 +82,7 @@ loadDFG("/tmp/savedgraph.tar.gz", IncrementalInference, dfg)
 ls(dfg)
 ```
 """
-function loadDFG(dst::String,
-                 iifModule::Module,
-                 dfgLoadInto::G) where G <: AbstractDFG
+function loadDFG!(dfgLoadInto::AbstractDFG, dst::String)
 
 
     #
@@ -151,7 +149,7 @@ function loadDFG(dst::String,
     # Finally, rebuild the CCW's for the factors to completely reinflate them
     @info "Rebuilding CCW's for the factors..."
     for factor in factors
-        iifModule.rebuildFactorMetadata!(dfgLoadInto, factor)
+        rebuildFactorMetadata!(dfgLoadInto, factor)
     end
 
     # remove the temporary unzipped file
diff --git a/src/LightDFG/entities/LightDFG.jl b/src/LightDFG/entities/LightDFG.jl
@@ -7,7 +7,7 @@ An in-memory DistributedFactorGraph based on LightGraphs.jl with parameters:
 - V: Variable type
 - F: Factor type
 """
-mutable struct LightDFG{T <: AbstractParams, V <: AbstractDFGVariable, F <:AbstractDFGFactor} <: AbstractDFG
+mutable struct LightDFG{T <: AbstractParams, V <: AbstractDFGVariable, F <:AbstractDFGFactor} <: AbstractDFG{T}
     g::FactorGraph{Int, V, F}
     description::String
     userId::String
diff --git a/src/entities/AbstractDFG.jl b/src/entities/AbstractDFG.jl
@@ -32,16 +32,15 @@ end
 
 """
 $(TYPEDEF)
-Abstract parent struct for a DFG graph.
+Abstract parent struct for solver parameters.
 """
-abstract type AbstractDFG
-end
+abstract type AbstractParams end
 
 """
 $(TYPEDEF)
-Abstract parent struct for solver parameters.
-"""
-abstract type AbstractParams end
+Abstract parent struct for a DFG graph.
+    """
+abstract type AbstractDFG{T<:AbstractParams} end
 
 """
 $(TYPEDEF)
diff --git a/src/services/AbstractDFG.jl b/src/services/AbstractDFG.jl
@@ -60,6 +60,12 @@ Method must be overloaded by the user for Serialization to work.  E.g. Increment
 getFactorOperationalMemoryType(dummy) = error("Please extend your workspace with function getFactorOperationalMemoryType(<:AbstractParams) for your usecase, e.g. IncrementalInference uses `CommonConvWrapper <: FactorOperationalMemory`")
 getFactorOperationalMemoryType(dfg::AbstractDFG) = getFactorOperationalMemoryType(getSolverParams(dfg))
 
+"""
+    $(SIGNATURES)
+
+Method must be overloaded by the user for Serialization to work.
+"""
+rebuildFactorMetadata!(dfg::AbstractDFG{<:AbstractParams}, factor::AbstractDFGFactor) = error("rebuildFactorMetadata! is not implemented for $(typeof(dfg))")
 
 ##------------------------------------------------------------------------------
 ## Setters
diff --git a/src/services/Serialization.jl b/src/services/Serialization.jl
@@ -167,7 +167,7 @@ function unpackFactor(dfg::G, packedProps::Dict{String, Any})::DFGFactor where G
     factor._variableOrderSymbols = _variableOrderSymbols
     setSolvable!(factor, solvable)
 
-    # Note, once inserted, you still need to call IIF.rebuildFactorMetadata!
+    # Note, once inserted, you still need to call rebuildFactorMetadata!
     return factor
 end
 
diff --git a/test/CGStructureTests.jl b/test/CGStructureTests.jl
@@ -1,10 +1,6 @@
 dfg = CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                             "testUser", "testRobot", "testSession",
                             "Description of test session",
-                            nothing,
-                            nothing,
-                            nothing,
-                            IncrementalInference.rebuildFactorMetadata!,
                             solverParams=SolverParams())
 
 # Nuke the user
diff --git a/test/IIF4DoorsTests.jl b/test/IIF4DoorsTests.jl
@@ -7,10 +7,6 @@ using IncrementalInference
 
 DistributedFactorGraphs.CloudGraphsDFG{SolverParams}() = CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                                                                     "testUser", "testRobot", "testSession",
-                                                                    nothing,
-                                                                    nothing,
-                                                                    IncrementalInference.decodePackedType,
-                                                                    IncrementalInference.rebuildFactorMetadata!,
                                                                     solverParams=SolverParams())
 
 N=100
diff --git a/test/consolInterfaceDev.jl b/test/consolInterfaceDev.jl
@@ -35,10 +35,6 @@ function DFG.CloudGraphsDFG(; hostname="localhost",
                                           robotId,
                                           sessionId,
                                           description,
-                                          nothing,
-                                          nothing,
-                                          nothing,#IncrementalInference.CommonConvWrapper,
-                                          (dfg,f)->f,#ncrementalInference.rebuildFactorMetadata!,
                                           solverParams=params)
 
     setUserData!(cgfg, Dict{Symbol, String}())
@@ -70,10 +66,6 @@ function DFG.CloudGraphsDFG(description::String,
                                                 robotId,
                                                 sessionId,
                                                 description,
-                                                nothing,
-                                                nothing,
-                                                nothing,#IncrementalInference.CommonConvWrapper,
-                                                (dfg,f)->f,#IncrementalInference.rebuildFactorMetadata!,
                                                 solverParams=solverParams)
 
 
@@ -207,6 +199,29 @@ end
     CopyFunctionsTest(testDFGAPI)
 
 end
+
+@testset "Copy Functions" begin
+
+    filename = "/tmp/fileDFG"
+
+    dfg, vars, facs = connectivityTestGraph(testDFGAPI)
+
+    saveDFG(dfg, filename)
+
+    copyDfg = DistributedFactorGraphs._getDuplicatedEmptyDFG(dfg)
+
+    @info "Going to load $filename"
+
+    loadDFG!(copyDfg, filename)
+
+    for var in vars
+        @test getVariable(dfg, var.label) == getVariable(copyDfg, var.label)
+    end
+    for fac in facs
+        @test getFactor(dfg, fac.label) == getFactor(copyDfg, fac.label)
+        # @test getFactor(dfg, fac.label) == fac
+    end
+end
 #
 #=
 fg = fg1
diff --git a/test/fileDFGTests.jl b/test/fileDFGTests.jl
@@ -35,11 +35,19 @@ using Test
 
         facts = map(n -> addFactor!(dfg, [verts[n], verts[n+1]], LinearConditional(Normal(50.0,2.0))), 1:(numNodes-1))
 
+        #FIXME a lot of these makes test fail
+        # facts[4].solverData.solveInProgress = 1
+        # facts[4].solverData.multihypo = [1, 0.1, 0.9]
+        # facts[4].solverData.eliminated = true
+        # facts[4].solverData.potentialused = true
+        # updateFactor!(dfg, facts[4])
         # Save and load the graph to test.
         saveDFG(dfg, filename)
 
         copyDfg = DistributedFactorGraphs._getDuplicatedEmptyDFG(dfg)
         @info "Going to load $filename"
+
+        #FIXME loadDFG!(copyDfg, filename), but use this to test deprecation
         retDFG = loadDFG(filename, Main, copyDfg) #, loaddir="/tmp")
 
         @test issetequal(ls(dfg), ls(retDFG))
diff --git a/test/iifInterfaceTests.jl b/test/iifInterfaceTests.jl
@@ -38,10 +38,6 @@ end
         dfg2 = CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                                             "testUser", "testRobot", "testSession2",
                                             "Description of test session 2",
-                                            nothing,
-                                            nothing,
-                                            nothing,
-                                            IncrementalInference.rebuildFactorMetadata!,
                                             solverParams=SolverParams())
     else
         dfg2 = T()
diff --git a/test/runtests.jl b/test/runtests.jl
@@ -90,10 +90,6 @@ if get(ENV, "IIF_TEST", "") == "true"
         CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                                     "testUser", "testRobot", "testSession",
                                     "Description of test Session",
-                                    nothing,
-                                    nothing,
-                                    nothing,
-                                    IncrementalInference.rebuildFactorMetadata!,
                                     solverParams=SolverParams())
             ]
     for api in apis
@@ -129,10 +125,6 @@ if get(ENV, "IIF_TEST", "") == "true"
             CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                                         "testUser", "testRobot", "simpleSolveSession",
                                         "Description of test Session",
-                                        nothing,
-                                        nothing,
-                                        IncrementalInference.decodePackedType,
-                                        IncrementalInference.rebuildFactorMetadata!,
                                         solverParams=SolverParams())
             ]
         for api in apis
@@ -146,10 +138,10 @@ else
 end
 
 
-struct NotImplementedDFG <: AbstractDFG end
+struct NotImplementedDFG{T} <: AbstractDFG{T} end
 
 @testset "No Interface tests" begin
-    dfg = NotImplementedDFG()
+    dfg = NotImplementedDFG{NoSolverParams}()
     v1 = SkeletonDFGVariable(:v1)
     f1 = SkeletonDFGFactor(:f1)
 
diff --git a/test/testBlocks.jl b/test/testBlocks.jl
@@ -64,12 +64,8 @@ TestCCW{T}() where T = TestCCW(T())
 
 Base.:(==)(a::TestCCW, b::TestCCW) = a.usrfnc! == b.usrfnc!
 
-
-## must overload the function
-import DistributedFactorGraphs: getFactorOperationalMemoryType
-
-# DF, dropping DFG. so that Main context is used and can be overridden by IIF.getFac...Type
-getFactorOperationalMemoryType(par::NoSolverParams) = TestCCW
+DFG.getFactorOperationalMemoryType(par::NoSolverParams) = TestCCW
+DFG.rebuildFactorMetadata!(dfg::AbstractDFG{NoSolverParams}, f::DFGFactor) = f
 
 function Base.convert(::Type{DFG.FunctionNodeData{TestCCW{F}}},
                      d::DFG.PackedFunctionNodeData{<:PackedInferenceType}) where F<:FunctorInferenceType
@@ -1008,7 +1004,7 @@ function connectivityTestGraph(::Type{T}; VARTYPE=DFGVariable, FACTYPE=DFGFactor
         setSolvable!(dfg, :x8, 0)
         setSolvable!(dfg, :x9, 0)
 
-        gfnd = GenericFunctionNodeData(false, false, Int[], TestCCW(TestFunctorInferenceType1()))
+        gfnd = GenericFunctionNodeData(true, true, Int[], TestCCW(TestFunctorInferenceType1()), Float64[], Int[], 1)
         f_tags = Set([:FACTOR])
         # f1 = DFGFactor(f1_lbl, [:a,:b], gfnd, tags = f_tags)
 
diff --git a/test/testFurther.jl b/test/testFurther.jl
@@ -4,10 +4,6 @@ using IncrementalInference
 dfg = CloudGraphsDFG{SolverParams}("localhost", 7474, "neo4j", "test",
                             "testUser", "testRobot", "testSession",
                             "description of test session",
-                            nothing,
-                            nothing,
-                            IncrementalInference.decodePackedType,
-                            IncrementalInference.rebuildFactorMetadata!,
                             solverParams=SolverParams())
 
 # Nuke the user
