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

[FEATURE] make VND.bw a 3 index array in preparation for parametric #465

Closed
dehann opened this issue Dec 5, 2019 · 7 comments
Closed

[FEATURE] make VND.bw a 3 index array in preparation for parametric #465

dehann opened this issue Dec 5, 2019 · 7 comments

Comments

@dehann
Copy link
Member

dehann commented Dec 5, 2019

Probably a DFG issue, but starting it here. Need is to store off-diagonal covariances while using the same VND data structure.

@dehann
Copy link
Member Author

dehann commented Dec 5, 2019

cc @Affie

@dehann
Copy link
Member Author

dehann commented Dec 5, 2019

we should look at what the performance characteristics would be.

@Affie
Copy link
Member

Affie commented Dec 6, 2019

I did a quick comparison test.
Array
0.026597 seconds (8 allocations: 22.888 MiB, 55.53% gc time)
Tuple
0.028543 seconds (8 allocations: 22.888 MiB, 29.23% gc time)
Struct
0.015862 seconds (8 allocations: 22.888 MiB, 23.56% gc time)
Mutable Struct
0.036606 seconds (1.00 M allocations: 38.147 MiB, 37.92% gc time)
I will look at the options when I have a bit of time

@Affie
Copy link
Member

Affie commented Dec 9, 2019

Starting of we are only working with one Gaussian mode. Therefore the covariance can fit in VND.bw,
I do not believe that an array is the easiest way forward. I propose that we wait with this changes that we might or might not need in the future. For instance, having a typed array might fit in a lot better when working with manifolds such as SE3. We can consider changing to types with a get/set index defined on them to make it easier to transition from the current usage.

DF, Agree lets think about doing a Vector of group elements for val and then supporting information like bw. Maybe these need to be nested types, although it's a little difficult to know precisely which type will work long term. Lets hold with bw::Array{<:Real,2} for now then.

@dehann
Copy link
Member Author

dehann commented Dec 16, 2019

Hi thanks for taking a look! Wait on accessors -- got it; and TypedArrays ... havent used or even heard of them myself, will go read up some more, thanks again.

@Affie
Copy link
Member

Affie commented Jan 6, 2020

What I meant was; for example an array of type SE3 Vector{SE3} and not Array{Float64,2}

@dehann
Copy link
Member Author

dehann commented Mar 19, 2020

see edit here: #465 (comment)

@dehann dehann closed this as completed Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants