Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility of loadDFG! with new module naming? #618

Closed
dehann opened this issue Aug 14, 2020 · 11 comments · Fixed by #620
Closed

Compatibility of loadDFG! with new module naming? #618

dehann opened this issue Aug 14, 2020 · 11 comments · Fixed by #620

Comments

@dehann
Copy link
Member

dehann commented Aug 14, 2020

HI @Affie , is it reasonable to require compatibility with new module names inside serialization? For example a FileDFG object that does not have any module information should be able to load like it always has?

Use this as a test: #582 (comment) which should load and give something similar to #582 (comment)

julia> fg = loadDFG("tenSolves.tar.gz")
sfolder = split(dstname, '.') = SubString{String}["tenSolves", "tar", "gz"]
[ Info: loadDFG! detected a gzip tenSolves.tar.gz -- unpacking via /tmp/caesar/random/94379f92 now...
┌ Error: Unable to deserialize soft type Pose2()
└ @ DistributedFactorGraphs ~/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:70
┌ Error: UndefVarError: Pose2() not defined
│ Stacktrace:
│  [1] getTypeFromSerializationModule(::String) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:67
│  [2] unpackVariableNodeData(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::DistributedFactorGraphs.PackedVariableNodeData) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:193
│  [3] #86 at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:125 [inlined]
│  [4] iterate at ./generator.jl:47 [inlined]
│  [5] collect(::Base.Generator{Base.ValueIterator{Dict{String,DistributedFactorGraphs.PackedVariableNodeData}},DistributedFactorGraphs.var"#86#87"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}}}) at ./array.jl:686
│  [6] map at ./abstractarray.jl:2188 [inlined]
│  [7] unpackVariable(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::Dict{String,Any}; unpackPPEs::Bool, unpackSolverData::Bool, unpackBigData::Bool) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:125
│  [8] unpackVariable at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:103 [inlined]
│  [9] (::DistributedFactorGraphs.var"#120#126"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}})(::IOStream) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:135
│  [10] open(::DistributedFactorGraphs.var"#120#126"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}}, ::String; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at ./io.jl:325
│  [11] open at ./io.jl:323 [inlined]
│  [12] loadDFG!(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::String) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:133
│  [13] loadDFG(::String) at /home/dehann/.julia/dev/IncrementalInference/src/FGOSUtils.jl:358
│  [14] top-level scope at REPL[3]:1
│  [15] eval(::Module, ::Any) at ./boot.jl:331
│  [16] eval_user_input(::Any, ::REPL.REPLBackend) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/REPL/src/REPL.jl:134
│  [17] repl_backend_loop(::REPL.REPLBackend) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/REPL/src/REPL.jl:195
│  [18] start_repl_backend(::REPL.REPLBackend, ::Any) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/REPL/src/REPL.jl:180
│  [19] run_repl(::REPL.AbstractREPL, ::Any; backend_on_current_task::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/REPL/src/REPL.jl:292
│  [20] run_repl(::REPL.AbstractREPL, ::Any) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/REPL/src/REPL.jl:288
│  [21] (::Base.var"#806#808"{Bool,Bool,Bool,Bool})(::Module) at ./client.jl:399
│  [22] #invokelatest#1 at ./essentials.jl:710 [inlined]
│  [23] invokelatest at ./essentials.jl:709 [inlined]
│  [24] run_main_repl(::Bool, ::Bool, ::Bool, ::Bool, ::Bool) at ./client.jl:383
│  [25] exec_options(::Base.JLOptions) at ./client.jl:313
│  [26] _start() at ./client.jl:506
└ @ DistributedFactorGraphs ~/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:74
ERROR: The variable doesn't seem to have a softtype. It needs to set up with an InferenceVariable from IIF. This will happen if you use DFG to add serialized variables directly and try use them. Please use IncrementalInference.addVariable().
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] unpackVariableNodeData(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::DistributedFactorGraphs.PackedVariableNodeData) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:194
 [3] #86 at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:125 [inlined]
 [4] iterate at ./generator.jl:47 [inlined]
 [5] collect(::Base.Generator{Base.ValueIterator{Dict{String,DistributedFactorGraphs.PackedVariableNodeData}},DistributedFactorGraphs.var"#86#87"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}}}) at ./array.jl:686
 [6] map at ./abstractarray.jl:2188 [inlined]
 [7] unpackVariable(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::Dict{String,Any}; unpackPPEs::Bool, unpackSolverData::Bool, unpackBigData::Bool) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:125
 [8] unpackVariable at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:103 [inlined]
 [9] (::DistributedFactorGraphs.var"#120#126"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}})(::IOStream) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:135
 [10] open(::DistributedFactorGraphs.var"#120#126"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}}, ::String; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at ./io.jl:325
 [11] open at ./io.jl:323 [inlined]
 [12] loadDFG!(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::String) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:133
 [13] loadDFG(::String) at /home/dehann/.julia/dev/IncrementalInference/src/FGOSUtils.jl:358
 [14] top-level scope at REPL[3]:1
@dehann dehann added this to the v0.10.0 milestone Aug 14, 2020
@Affie
Copy link
Member

Affie commented Aug 14, 2020

Do you mean you want backward compatibility?

@dehann
Copy link
Member Author

dehann commented Aug 14, 2020

Yes, is that something reasonable, or just way too crazy?

@dehann
Copy link
Member Author

dehann commented Aug 14, 2020

it's not super serious, but lets at least actively decide to include or exclude that kind of compatibility? I'm like 80% don't be compatible, until we learn otherwise?

@Affie
Copy link
Member

Affie commented Aug 14, 2020

@GearsAD had some ideas of how to convert files saved in for instance v0.9 to v0.10. Can’t exactly remember the details though.

@Affie
Copy link
Member

Affie commented Aug 14, 2020

Its been breaking with every minor so far. Thats why asked for test and versions that Sam is implementing.

@dehann
Copy link
Member Author

dehann commented Aug 14, 2020

problem is too many breaking changes at once requiring blind updates to code for a few hours before I'm able to make progress again. e.g. when trying to make a new fg tar to fix serialization change, i also get a new error between yesterday and today:

LoadError: no promotion exists for DateTime and TimeZones.ZonedDateTime

updates to downstream code has to happen anyway, just not a smooth transition from DFG v0.9 to v0.10 unfortunately. Maybe next cycle.

@Affie
Copy link
Member

Affie commented Aug 14, 2020

LoadError: no promotion exists for DateTime and TimeZones.ZonedDateTime

I think it was removed from deprecated too soon. Maybe just put it back.

That was changes from v0.8 to v0.9

@dehann
Copy link
Member Author

dehann commented Aug 14, 2020

sounds good thanks!

@Affie
Copy link
Member

Affie commented Aug 14, 2020

@dehann On second thought. Your data should work. If the softtype doesn’t have a module it should look in Main like before. Can you upload the file somewhere. Then I’ll check.

Edit: sorry i see you do refer to file. I’ll test it.

@Affie
Copy link
Member

Affie commented Aug 14, 2020

I had a look on my phone and the issue comes from inconsistencies in how the softtype used to be serialized. Its different in the variable and variable node data. So can do easy patch that we can remove later. The only limitation might be that it has to be the new stateless softtypes.

@dehann
Copy link
Member Author

dehann commented Aug 15, 2020

i agree with your suggestion, and would add that we should permanently do automation inside IIF.addVariable! IIF.addFactor! to promote ::DateTime to ::ZonedDateTime(dt, localzone())

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants