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

Mixture is failing to add in Python tutorial 2 #25

Closed
GearsAD opened this issue Mar 23, 2022 · 14 comments
Closed

Mixture is failing to add in Python tutorial 2 #25

GearsAD opened this issue Mar 23, 2022 · 14 comments
Assignees

Comments

@GearsAD
Copy link
Contributor

GearsAD commented Mar 23, 2022

Payload is failing to add, but the content looks to be correct. We should investigate unpacking and resolve.

This is the python packed factor payload:

{"label": "x1x2f1", "_version": "0.18.1", "_variableOrderSymbols": ["x1", "x2"], "data": {"eliminated": false, "potentialused": false, "edgeIDs": [], "fnc": {"N": 2, "F_": "PackedLinearRelative", "S": ["hypo1", "hypo2"], "components": [{"_type": "IncrementalInference.PackedRayleigh", "sigma": 3.0}, {"_type": "IncrementalInference.PackedUniform", "a": 30.0, "b": 55.0, "PackedSamplableTypeJSON": "IncrementalInference.PackedUniform"}], "diversity": {"_type": "IncrementalInference.PackedCategorical", "p": [0.4, 0.6]}}, "multihypo": [], "certainhypo": [1, 2], "nullhypo": 0.0, "solveInProgress": 0, "inflation": 5.0}, "tags": ["FACTOR"], "timestamp": "2022-03-23T12:21:24.554Z", "nstime": "0", "fnctype": "Mixture", "solvable": 1}
@GearsAD GearsAD transferred this issue from NavAbility/NavAbilitySDK.py Mar 23, 2022
@jim-hill-r
Copy link
Contributor

Could you drop the couple of lines of Caesar code that produces this factor in a comment? I can write a unit test against the cloud and see what happens.

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 23, 2022

Yep I checked the payloads but can do.

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 23, 2022

using IncrementalInference
using DistributedFactorGraphs

fg = initfg()

# add the first node
addVariable!(fg, :x0, ContinuousScalar)

addFactor!(fg, [:x0], Prior(Normal(0,1)))
addVariable!(fg, :x1, ContinuousScalar)
# P(Z | :x1 - :x0 ) where Z ~ Normal(10,1)
addFactor!(fg, [:x0, :x1], LinearRelative(Normal(10.0,1)))

addVariable!(fg, :x2, ContinuousScalar)
mmo = Mixture(LinearRelative, 
              (hypo1=Rayleigh(3), hypo2=Uniform(30,55)), 
              [0.4; 0.6])
addFactor!(fg, [:x1, :x2], mmo)

# This factor - x1x2f1 is the one that is not loading.
f = getFactor(fg, :x1x2f1)

@jim-hill-r jim-hill-r self-assigned this Mar 23, 2022
@jim-hill-r
Copy link
Contributor

I'll take a look tonight.

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 23, 2022

Thanks! I see a decoding issue in the worker logs, but the payloads look the same.

# Julia
{"label":"x1x2f1","_version":"0.18.1","_variableOrderSymbols":["x1","x2"],"data":{"eliminated":false,"potentialused":false,"edgeIDs":[],"fnc":{"N":2,"F_":"PackedLinearRelative","S":["hypo1","hypo2"],"components":[{"_type":"IncrementalInference.PackedRayleigh","sigma":3.0},{"_type":"IncrementalInference.PackedUniform","a":30.0,"b":55.0,"PackedSamplableTypeJSON":"IncrementalInference.PackedUniform"}],"diversity":{"_type":"IncrementalInference.PackedCategorical","p":[0.4,0.6]}},"multihypo":[],"certainhypo":[1,2],"nullhypo":0.0,"solveInProgress":0,"inflation":5.0},"tags":["FACTOR"],"timestamp":"2022-03-23T07:09:55.020-05:00","nstime":"0","fnctype":"Mixture","solvable":1}
# Python
{"label":"x1x2f1","_version":"0.18.1","_variableOrderSymbols":["x1","x2"],"data":{"eliminated":false,"potentialused":false,"edgeIDs":[],"fnc":{"N":2,"F_":"PackedLinearRelative","S":["hypo1","hypo2"],"components":[{"_type":"IncrementalInference.PackedRayleigh","sigma":3.0},{"_type":"IncrementalInference.PackedUniform","a":30.0,"b":55.0,"PackedSamplableTypeJSON":"IncrementalInference.PackedUniform"}],"diversity":{"_type":"IncrementalInference.PackedCategorical","p":[0.4,0.6]}},"multihypo":[],"certainhypo":[1,2],"nullhypo":0.0,"solveInProgress":0,"inflation":5.0},"tags":["FACTOR"],"timestamp":"2022-03-23T12:21:24.554Z","nstime":"0","fnctype":"Mixture","solvable":1}

@jim-hill-r
Copy link
Contributor

TIMESTAMPS!!!

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 23, 2022

Really?

@jim-hill-r
Copy link
Contributor

'Z' is the pattern we have used in the SDK, but maybe the Mixtures code didn't get upgraded to Timezones.jl along with the rest. Not sure the best way to fix this. @Affie Advise?

@dehann
Copy link
Contributor

dehann commented Mar 23, 2022

but maybe the Mixtures code didn't get upgraded to Timezones.jl along with the rest.

The odds of that are low, but I cannot say with certainty 'no'. An error stack would be quick to resolve. By looking blind at the code, I'd be curious if the problem happens above, below, or inside this line:
https://github.com/JuliaRobotics/IncrementalInference.jl/blob/0f63b500191e35b3ec34f86bbe1185fce756ee80/src/Factors/Mixture.jl#L143

Nothing here suggests that the Timestamp would give an error.

Longer term though, it does speak to need for resolving:


Tried Two things (FileDFG and Copy-Paste Bug into...)

  • So I also tried Sam's example above via FileDFG, and that seemed to work fine.
  • Then I modified the FileDFG tar.gz by copying in the Z timestamp string above (into factors/x1x2f1.json), but loads fine too. Here is the "test_bug" FileDFG:

Also, I think the Z timestamp is generated by SDK.py. I think we should support both timestamp types in DFG/IIF?

json string with "-07:00" test.tar.gz

julia> fg
GraphsDFG{SolverParams, DFGVariable, DFGFactor}
  UserId: DefaultUser
  RobotId: DefaultRobot
  SessionId: Session_7c4118
  Description: Graphs.jl implementation
  Nr variables: 3
  Nr factors: 3
  User Data: Symbol[]
  Robot Data: Symbol[]
  Session Data: Symbol[]


julia> saveDFG("test.tar.gz", fg)

julia> fg_ = loadDFG("test.tar.gz")
sfolder = split(dstname, '.') = SubString{String}["test", "tar", "gz"]
[ Info: loadDFG! detected a gzip test.tar.gz -- unpacking via /tmp/caesar/random/e88cb92c now...
[ Info: Loaded 3 variables - [:x0, :x1, :x2]
[ Info: Inserting variables into graph...
┌ Warning: Unable to create measurements and gradients for LinearRelative{1, Normal{Float64}}(Normal{Float64}=10.0, σ=1.0)) during prep of CCW, falling back on no-partial information assumption.  Enable ENV["JULIA_DEBUG"] = "IncrementalInference" for @debug printing to see the error.
└ @ IncrementalInference ~/.julia/dev/IncrementalInference/src/services/CalcFactor.jl:405
[ Info: Loaded 3 factors - [:x0f1, :x0x1f1, :x1x2f1]
[ Info: Inserting factors into graph...
[ Info: Rebuilding CCW's for the factors...
┌ Warning: Unable to create measurements and gradients for LinearRelative{1, Normal{Float64}}(Normal{Float64}=10.0, σ=1.0)) during prep of CCW, falling back on no-partial information assumption.  Enable ENV["JULIA_DEBUG"] = "IncrementalInference" for @debug printing to see the error.
└ @ IncrementalInference ~/.julia/dev/IncrementalInference/src/services/CalcFactor.jl:405
[ Info: DFG.loadDFG! is deleting a temp folder created during unzip, /tmp/caesar/random/e88cb92c
GraphsDFG{SolverParams, DFGVariable, DFGFactor}
  UserId: DefaultUser
  RobotId: DefaultRobot
  SessionId: Session_763cdb
  Description: Graphs.jl implementation
  Nr variables: 3
  Nr factors: 3
  User Data: Symbol[]
  Robot Data: Symbol[]
  Session Data: Symbol[]

json string with Z test_bug.tar.gz

julia> fg_ = loadDFG("test_bug.tar.gz")
sfolder = split(dstname, '.') = SubString{String}["test_bug", "tar", "gz"]
[ Info: loadDFG! detected a gzip test_bug.tar.gz -- unpacking via /tmp/caesar/random/0057b7c2 now...
[ Info: Loaded 3 variables - [:x0, :x1, :x2]
[ Info: Inserting variables into graph...
┌ Warning: Unable to create measurements and gradients for LinearRelative{1, Normal{Float64}}(Normal{Float64}=10.0, σ=1.0)) during prep of CCW, falling back on no-partial information assumption.  Enable ENV["JULIA_DEBUG"] = "IncrementalInference" for @debug printing to see the error.
└ @ IncrementalInference ~/.julia/dev/IncrementalInference/src/services/CalcFactor.jl:405
[ Info: Loaded 3 factors - [:x0f1, :x0x1f1, :x1x2f1]
[ Info: Inserting factors into graph...
[ Info: Rebuilding CCW's for the factors...
┌ Warning: Unable to create measurements and gradients for LinearRelative{1, Normal{Float64}}(Normal{Float64}=10.0, σ=1.0)) during prep of CCW, falling back on no-partial information assumption.  Enable ENV["JULIA_DEBUG"] = "IncrementalInference" for @debug printing to see the error.
└ @ IncrementalInference ~/.julia/dev/IncrementalInference/src/services/CalcFactor.jl:405
[ Info: DFG.loadDFG! is deleting a temp folder created during unzip, /tmp/caesar/random/0057b7c2
GraphsDFG{SolverParams, DFGVariable, DFGFactor}
  UserId: DefaultUser
  RobotId: DefaultRobot
  SessionId: Session_1f664a
  Description: Graphs.jl implementation
  Nr variables: 3
  Nr factors: 3
  User Data: Symbol[]
  Robot Data: Symbol[]
  Session Data: Symbol[]

@jim-hill-r
Copy link
Contributor

Let's write the test and validate the "red herring" assumption that timestamps are the issue.

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 24, 2022

No I believe this is a serialization issue unrelated to timestamps. More to follow in a few moments.

@GearsAD
Copy link
Contributor Author

GearsAD commented Mar 24, 2022

It looks to be related to the way the Dict{string, Any} are converted to NamedTuples that are used to create the distributions and the PackedMixture model. We can discuss in morning.

@dehann dehann self-assigned this Mar 24, 2022
@dehann
Copy link
Contributor

dehann commented Mar 24, 2022

Right, there were two bugs.

DFG is using JSON2 to unpack the json strings.

That is why FileDFG was able to unpack the bug string above. Sam came up against differences between JSON and JSON2 when trying this yesterday. DFG is using JSON2.

The right way to unpack factors currently is:
https://github.com/JuliaRobotics/DistributedFactorGraphs.jl/blob/24ef7c067ff2de4a069070a43dab939f8bbe5d30/src/FileDFG/services/FileDFG.jl#L145-L146

Cloud was not yet on IIF v0.27.3


Aside, I have a good idea of how to support both JSON and JSON2. Thats happening here:

@dehann
Copy link
Contributor

dehann commented Mar 24, 2022

Think we can close this now.

@dehann dehann closed this as completed Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants