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

poses vector grows indefinitely #122

Closed
ahans opened this issue Apr 4, 2023 · 2 comments
Closed

poses vector grows indefinitely #122

ahans opened this issue Apr 4, 2023 · 2 comments
Assignees

Comments

@ahans
Copy link
Contributor

ahans commented Apr 4, 2023

The poses vector is only ever added to. While poses are small objects, eventually any system will run out of memory. We need some bound for this.

To fix this, we could define some number of last poses that are kept. However, it looks like currently only the very first and last two poses are ever used. An even better fix might therefore be just keeping those.

@tizianoGuadagnino
Copy link
Collaborator

Hi @ahans,
The unbounded growth of the pose vector it is not a sensible problem in our point of view. A pose using Sophus is represented by 7 doubles. Unless there are specific memory restrictions, you need an extremely high number of poses for this to be a real problem. Furthermore, we need to store the all trajectory so that it can be inspected after a run.

@ahans
Copy link
Contributor Author

ahans commented Apr 14, 2023

Keeping all poses is just nothing we need in KISS-ICP core. If you want to inspect them after a run, there should be another place where you collect the poses. Maybe somewhere in the Python driver. I was under the impression you wanted to do things properly in this project, and unbounded growth is simply tech debt. And in the present case it's actually quite simple to fix it, as shown in my PR #124. Granted, the Python side would need more some work, but it's still small enough to not accept this. But all of that is IMHO. If you want to keep this as is, fine.

tizianoGuadagnino added a commit that referenced this issue Apr 12, 2024
…ectory. Fixes #122 (#318)

* First draft for fixed_size_vector. Fixes #122

* Pose pair, many ifs go Rhaus if this works

* New adaptive threshold based on pose vector modification

* Now has moved also rhaus from python

* Push new draft with no new types

* rename

* Fix ROS2

* Remove unused functions from cpp

* Reduce code lines more

* Change names and add some more comments

* Marry python and C++ implementations

* remove unused headers

* Keep copy-pasting 🤦

* Move poses array one level up, and add new deskew function to python API

Now I like it even better. The `pipeline` is in charge of managing the
trajectory of poses, as the KISS ICP method is invariant to this.

* Renaming of Registration types and functions

* from current to last

* Renaming and reshaping in Adaptive threshold

* More cosmesis on the AdaptiveThreshold

* Remove constexpr

* Ciao deskew with start/end pose

* At least inline it

* Const correcteness

* New ATE from Benedikt PR

* Revert "New ATE from Benedikt PR"

This reverts commit af201c4.

---------

Co-authored-by: tizianoGuadagnino <frevo93@gmail.com>
Co-authored-by: Benedikt Mersch <benedikt.mersch@gmail.com>
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

2 participants