-
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
Cleanup defVariable and add getPointType #775
Conversation
c0393ff
to
c2ae4df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, we must just do the integration testing with downstream. Thanks for fixing the tests too!
Should work with these changes: JuliaRobotics/IncrementalInference.jl#1275 I haven't tested rome |
julia> using DistributedFactorGraphs
julia> using IncrementalInference
[ Info: Precompiling IncrementalInference [904591bb-b899-562f-9e6f-b8df64c7d480]
[ Info: KernelDensityEstimate.FORCE_EVAL_DIRECT = true
ERROR: LoadError: LoadError: LoadError: LoadError: MethodError: no method matching var"@defVariable"(::LineNumberNode, ::Module, ::Symbol, ::Expr)
Closest candidates are:
var"@defVariable"(::LineNumberNode, ::Module, ::Any, ::Any, ::Any) at /home/dehann/.julia/dev/DistributedFactorGraphs/src/services/DFGVariable.jl:89
Stacktrace:
[1] #macroexpand#50
@ ./expr.jl:112 [inlined]
[2] macroexpand
@ ./expr.jl:111 [inlined]
[3] docm(source::LineNumberNode, mod::Module, meta::Any, ex::Any, define::Bool) (repeats 2 times)
@ Base.Docs ./docs/Docs.jl:537
[4] (::DocStringExtensions.var"#29#30"{typeof(DocStringExtensions.template_hook)})(::LineNumberNode, ::Vararg{Any, N} where N)
@ DocStringExtensions ~/.julia/packages/DocStringExtensions/9Ot5N/src/templates.jl:11
[5] var"@doc"(__source__::LineNumberNode, __module__::Module, x::Vararg{Any, N} where N)
@ Core ./boot.jl:508
[6] include(mod::Module, _path::String)
@ Base ./Base.jl:386
[7] include(x::String)
@ IncrementalInference ~/.julia/dev/IncrementalInference/src/IncrementalInference.jl:1
[8] top-level scope
@ ~/.julia/dev/IncrementalInference/src/IncrementalInference.jl:427
[9] include
@ ./Base.jl:386 [inlined]
[10] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::Nothing)
@ Base ./loading.jl:1213
[11] top-level scope
@ none:1
[12] eval
@ ./boot.jl:360 [inlined]
[13] eval(x::Expr)
@ Base.MainInclude ./client.jl:446
[14] top-level scope
@ none:1
in expression starting at /home/dehann/.julia/dev/IncrementalInference/src/Variables/DefaultVariables.jl:23
in expression starting at /home/dehann/.julia/dev/IncrementalInference/src/Variables/DefaultVariables.jl:15
in expression starting at /home/dehann/.julia/dev/IncrementalInference/src/Variables/DefaultVariables.jl:15
in expression starting at /home/dehann/.julia/dev/IncrementalInference/src/IncrementalInference.jl:1
ERROR: Failed to precompile IncrementalInference [904591bb-b899-562f-9e6f-b8df64c7d480] to /home/dehann/.julia/compiled/v1.6/IncrementalInference/jl_alznRu.
Stacktrace:
[1] error(s::String)
@ Base ./error.jl:33
[2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::Base.TTY, internal_stdout::Base.TTY)
@ Base ./loading.jl:1360
[3] compilecache(pkg::Base.PkgId, path::String)
@ Base ./loading.jl:1306
[4] _require(pkg::Base.PkgId)
@ Base ./loading.jl:1021
[5] require(uuidkey::Base.PkgId)
@ Base ./loading.jl:914
[6] require(into::Module, mod::Symbol)
@ Base ./loading.jl:901 Latest branches for DFG (this 775) and IIF (1275): (@v1.6) pkg> st DistributedFactorGraphs IncrementalInference
Status `~/.julia/environments/v1.6/Project.toml`
[b5cc3c7e] DistributedFactorGraphs v0.14.5 `~/.julia/dev/DistributedFactorGraphs`
[904591bb] IncrementalInference v0.24.4 `~/.julia/dev/IncrementalInference` |
I'm busy adding the point types on IIF 1275 |
I forgot to commit @defVariable changes, just did it now. JuliaRobotics/IncrementalInference.jl#1275 (comment) |
I figured we first update DFG and iif with the new Vector{T} and then look at RoME, as not to break all the masters at once. |
getPointType