-
Notifications
You must be signed in to change notification settings - Fork 2
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
loadDFG! error, "type does not have a definite number of fields", BlobStoreEntry #582
Comments
captured here #557 |
right thanks, so the data |
Yes, part of my ongoing pain with serialization. If I remember correctly it was the addition of ZonedTimeStamp that eventually broke it. |
I'm tempted to change everything to use the pattern in https://github.com/JuliaData/StructTypes.jl + JSON3 |
Think we have other bigger issues to resolve first? We keep fixing serialization stuff over and over, so lets talk next week on what the most important things are too fix with serialization and balance that with what progress is required on other fronts. I'd say hold off on JSON3 stuff for now, if it can.
Haha, jip I believe it. The repercussions of small changes in DFG are dramatic downstream. From that quick ZonedDateTime change in #517 :-) At least this is a good test case for current dev work. |
Actually that one is broken in another way: #560 I'm speaking about this one:
|
Another thing, we actively decided after a 2 year process not to define the "schema" for serialization. We originally had a protobuf packing approach which was hard schema type dependent and which supported forward-backward multilanguage compatibility. After much pain, we decided that top level packing MUST be JSON, and that the JSON string must at top level include the format in which the data is serialized. This is to mimic how the Internet has standardized it's data exchange format. So So to stress, its probably not a good idea going down a path to unilaterally "change everything" on serialization. In this case, it will be better to understand what the problems are with JSON.jl JSON2.jl JSON3.jl, because clearly they haven't figured it all out either. But we likely won't go down writing JSON4.jl either. Also remember that the whole point of JSON is that serialization should be marginally or completely available outside Julia. The ZMQ interface for example is about multilanguage support and it uses the same serialization. The DB storage is definitely not a Julia implementation, later we will add a FuseDFG driver, etc, etc. A little while back we came up with the requirement that ZMQ messages should be exactly the same as FileDFG content. The schema discussion becomes a whole issue there. There are 100 different schema's available... So serialization only has 2nd best answers for everything. Whenever you pick the "best" answer for one requirement, then all others get real upset. So we have to accept a 2nd best solution for everybody and JSON is the best bet at this time. Again, we should discuss this in the upcoming call, but can guarantee a schema'ed serialization approach is a touchy subject. cc @GearsAD |
Definitely worth discussing on Friday. Is this still an issue? |
now on Julia 1.5.0 Jip, im trying to debug a bit, but not familiar enough with this part of the code yet. I modified Serialization.jl Line 75 to print: for (k,bdeInter) in dataIntermed
@show bdeInter
@show dataElemTypes[k], getfield(DistributedFactorGraphs, dataElemTypes[k])
fullVal = JSON2.read(bdeInter, getfield(DistributedFactorGraphs, dataElemTypes[k]))
variable.dataDict[k] = fullVal
end Gives this on the data uploaded in the first comment: (dataElemTypes[k], getfield(DistributedFactorGraphs, dataElemTypes[k])) = (:BlobStoreEntry, DistributedFactorGraphs.BlobStoreEntry)
ERROR: ArgumentError: type does not have a definite number of fields
Stacktrace:
[1] fieldcount(::Any) at ./reflection.jl:722
[2] #s17#65 at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:443 [inlined]
[3] #s17#65(::Any, ::Any, ::Any, ::Any, ::Any, ::Any) at ./none:0
[4] (::Core.GeneratedFunctionStub)(::Any, ::Vararg{Any,N} where N) at ./boot.jl:527
[5] read at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:442 [inlined]
[6] macro expansion at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:259 [inlined]
[7] read(::Base.GenericIOBuffer{Array{UInt8,1}}, ::Type{TimeZones.ZonedDateTime}; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:442
[8] read at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:442 [inlined]
[9] macro expansion at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:259 [inlined]
[10] read(::Base.GenericIOBuffer{Array{UInt8,1}}, ::Type{DistributedFactorGraphs.BlobStoreEntry}; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:442
[11] read(::Base.GenericIOBuffer{Array{UInt8,1}}, ::Type{DistributedFactorGraphs.BlobStoreEntry}) at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:442
[12] read(::String, ::Type{T} where T; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:36
[13] read(::String, ::Type{T} where T) at /home/dehann/.julia/packages/JSON2/ld4Kq/src/read.jl:36
[14] 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:78
[15] unpackVariable at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:31 [inlined]
[16] (::DistributedFactorGraphs.var"#118#124"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}})(::IOStream) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:134
[17] open(::DistributedFactorGraphs.var"#118#124"{LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor},Array{DistributedFactorGraphs.DFGVariable,1}}, ::String; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at ./io.jl:325
[18] open at ./io.jl:323 [inlined]
[19] loadDFG!(::LightDFG{SolverParams,DistributedFactorGraphs.DFGVariable,DistributedFactorGraphs.DFGFactor}, ::String) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/FileDFG/services/FileDFG.jl:132
[20] top-level scope at none:1 Some example data: Debug: DECODING Softtype = Pose2
└ @ DistributedFactorGraphs ~/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:117
bdeInter = "{\"label\":\"JOYSTICK_CMD_VALS\",\"id\":{\"value\":56433955245256332240062641469242912422},\"blobstore\":\"default_folder_store\",\"hash\":\"fc2c9f7456ddd63e6b77b558381d72d82f0b839fb6c4cb6f5ea6aa44cb8fd783\",\"origin\":\"DefaultUser|DefaultRobot|Session_4071d1|x0\",\"description\":\"\",\"mimeType\":\"\",\"createdTimestamp\":{\"utc_datetime\":\"2020-08-11T05:37:37.241\",\"timezone\":{\"name\":\"America/New_York\",\"transitions\":[{\"utc_datetime\":\"8511-01-01T00:00:00.0\",\"zone\":{\"name\":\"LMT\",\"offset\":{\"std\":{\"value\":-17762},\"dst\":{\"value\":0}}}},{\"utc_datetime\":\"1883-11-18T17:00:00.0\",\"zone\":{\"name\":\"EST\",\"offset\":{\"std\":{\"value\":-18000},\"dst\":{\"value\":0}}}},{\"utc_datetime\":\"1918-03-31T07:00:00.0\",\"zone\":{\"name\":\"EDT\",\"offset\":{\"std\":{\"value\":-18000}, |
This is due to the ZonedTimestamp being serialized in as-is into the structure. If you check one of the files, the |
Okay, load did not work with #588 merged, but let me create a new tar.gz and see if that does the trick... |
just pointing to the link again |
All the x variables still have that issue. I'm looking for a quick fix. |
newfinalGraph.tar.gz this was created with #588 included |
nope unfortunately still the same bug -- can continue with this tomorrow or later in the week. Don't worry about pushing too late tonight. Thanks for looking! |
Okay so there's two problems here:
DiscussionI'd recommend using JSON and Unmarshal here, rather than JSON2. JSON has a lot more flexibility with preparsing, and it does at least handle the createdTimestamp correctly. The UUID, we need a custom parser for it. The start of saving it correctly:
Gives (note the UUID is still wrong):
To parse back:
Proposed FixOkay, if it's not 100% urgent tonight I better head out, but will look tomorrow morning again. Current things to do:
This recovers the correct data:
|
Cool thanks, this is plenty of info thanks -- head out and we'll catch up another time! |
Can you try this? #589 |
running new solve... will take 5-10mins |
loading the old file (expected to fail) does but with a different error -- think that has to do with one of the items you mentioned earlier. New solve is under way I'll try as fast as possible. It's about halfway now... Thanks for pushing a fix so quick! |
MWE of the current issue: using Unmarshal, JSON
struct MyType
createdTimestamp::ZonedDateTime
end
str = "{\"createdTimestamp\":\"2020-08-11T04:05:59.82-04:00\"}"
interm = JSON.parse(str)
# Dict{String,Any} with 1 entry:
# "createdTimestamp" => "2020-08-11T04:05:59.82-04:00"
Unmarshal.unmarshal(MyType, interm)
ERROR: ArgumentError: Unable to parse string "2020-08-11T04:05:59.82-04:00" using format dateformat"yyyy-mm-ddTHH:MM:SS.ssszzz". Unable to parse date time. Expected directive DatePart(sss) at char 21
Stacktrace:
[1] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Dates/src/parse.jl:104 [inlined]
[2] tryparsenext_core(::String, ::Int64, ::Int64, ::Dates.DateFormat{Symbol("yyyy-mm-ddTHH:MM:SS.ssszzz"),Tuple{Dates.DatePart{'y'},Dates.Delim{Char,1},Dates.DatePart{'m'},Dates.Delim{Char,1},Dates.DatePart{'d'},Dates.Delim{Char,1},Dates.DatePart{'H'},Dates.Delim{Char,1},Dates.DatePart{'M'},Dates.Delim{Char,1},Dates.DatePart{'S'},Dates.Delim{Char,1},Dates.DatePart{'s'},Dates.DatePart{'z'}}}, ::Bool) at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Dates/src/parse.jl:38
[3] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Dates/src/parse.jl:150 [inlined]
[4] tryparsenext_internal at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Dates/src/parse.jl:125 [inlined]
[5] parse at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.5/Dates/src/parse.jl:282 [inlined]
[6] ZonedDateTime(::String, ::Dates.DateFormat{Symbol("yyyy-mm-ddTHH:MM:SS.ssszzz"),Tuple{Dates.DatePart{'y'},Dates.Delim{Char,1},Dates.DatePart{'m'},Dates.Delim{Char,1},Dates.DatePart{'d'},Dates.Delim{Char,1},Dates.DatePart{'H'},Dates.Delim{Char,1},Dates.DatePart{'M'},Dates.Delim{Char,1},Dates.DatePart{'S'},Dates.Delim{Char,1},Dates.DatePart{'s'},Dates.DatePart{'z'}}}) at /home/dehann/.julia/packages/TimeZones/v0mfN/src/parse.jl:88
[7] ZonedDateTime(::String) at /home/dehann/.julia/packages/TimeZones/v0mfN/src/parse.jl:87
[8] unmarshal(::Type{T} where T, ::String, ::Bool, ::Int64) at /home/dehann/.julia/packages/Unmarshal/QoOc6/src/Unmarshal.jl:38
[9] unmarshal(::Type{T} where T, ::Dict{String,Any}, ::Bool, ::Int64) at /home/dehann/.julia/packages/Unmarshal/QoOc6/src/Unmarshal.jl:113
[10] unmarshal(::Type{T} where T, ::Dict{String,Any}) at /home/dehann/.julia/packages/Unmarshal/QoOc6/src/Unmarshal.jl:85
[11] top-level scope at REPL[10]:1 |
BUT, this works: julia> str = "{\"createdTimestamp\":\"2020-08-11T04:05:59.820-04:00\"}"
"{\"createdTimestamp\":\"2020-08-11T04:05:59.820-04:00\"}"
julia> interm = JSON.parse(str)
Dict{String,Any} with 1 entry:
"createdTimestamp" => "2020-08-11T04:05:59.820-04:00"
julia> Unmarshal.unmarshal(MyType, interm)
MyType(ZonedDateTime(2020, 8, 11, 4, 5, 59, 820, tz"UTC-04:00")) So problem really is that when milliseconds are written with one or two characters rather than the expected 3: |
Okay, lets try this, will add to PR #589 soon: # Corrects any `::ZonedDateTime` fields of T in corresponding `interm::Dict` as `dateformat"yyyy-mm-ddTHH:MM:SS.ssszzz"`
function standardizeZDTString!(T, interm::Dict)
@debug "About to look through types of" T
for (name, typ) in zip(fieldnames(T), T.types)
@debug "name=$name"
@debug "typ=$typ"
if typ <: ZonedDateTime
@debug "must ensure SS.ssszzz on $name :: $typ"
namestr = string(name)
supersec, subsec = split(interm[namestr], '.')
sss, zzz = split(subsec, '-')
@debug "split time elements are $sss-$zzz"
# make sure milliseconds portion is precisely 3 characters long
if length(sss) < 3
# pad with zeros at the end
while length(sss) < 3
sss *= "0"
end
newtimessszzz = supersec*"."*sss*"-"*zzz
@debug "new time string: $newtimessszzz"
# reassembled ZonedDateTime is put back in the dict
interm[namestr] = newtimessszzz
end
end
end
nothing
end |
Wait, Sam's is so much better: # Make sure that the timestamp is correctly formatted with subseconds
packedProps["timestamp"] = replace(packedProps["timestamp"], r":(\d)(\d)(Z|z|\+|-)" => s":\1\2.000\3") |
@debug "must ensure SS.ssszzz on $name :: $typ -- $(interm[namestr])"
# Make sure that the timestamp is correctly formatted with subseconds, copy #588
interm[namestr] = replace(interm[namestr], r":(\d)(\d)(Z|z|\+|-)" => s":\1\2.000\3")
@debug "after SS.ssszzz -- $(interm[namestr])" That didnt work? ┌ Debug: must ensure SS.ssszzz on createdTimestamp :: TimeZones.ZonedDateTime -- 2020-08-11T04:05:59.82-04:00
└ @ DistributedFactorGraphs ~/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:44
┌ Debug: after SS.ssszzz -- 2020-08-11T04:05:59.82-04:00
└ @ DistributedFactorGraphs ~/.julia/dev/DistributedFactorGraphs/src/services/Serialization.jl:46
ERROR: ArgumentError: Unable to parse string "2020-08-11T04:05:59.82-04:00" using format dateformat"yyyy-mm-ddTHH:MM:SS.ssszzz". Unable to parse date time. Expected directive DatePart(sss) at char 21 |
I'm a bit confused about all the timestamp functions such as (#582 (comment)) was this not sufficient?
However, I don't like the idea of manually having to functions to serialise simple types such as these. That's why the JSON3 comment. It's definitely worth a discussion to sort out properly. |
Doesn't seem to work. julia> str = "2020-08-11T04:05:59.82-04:00"
"2020-08-11T04:05:59.82-04:00"
julia> replace(str, r":(\d)(\d)(Z|z|\+|-)" => s":\1\2.000\3")
"2020-08-11T04:05:59.82-04:00" |
we can use (refactor) the new |
Didn’t see the functions to standardize the strings, I used a regex in #588 , if there’s a standard fix will move to that. Net takeaway is actually timezones.jl is way too strict on time stamp format |
If it weren't for these time zones, we would have been in the same time zone and I could have told you about this time zone issue. |
Hahaha! |
finalGraph.tar.gz
On Julia 1.4.2
The text was updated successfully, but these errors were encountered: