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

Use double buffering in particle storage #266

Merged
merged 3 commits into from
Nov 6, 2023

Conversation

hidmic
Copy link
Collaborator

@hidmic hidmic commented Nov 5, 2023

Proposed changes

Pretty much what the title says. This patch fully initializes a particle cloud in the background before replacing the one in foreground.

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

Checklist

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

Additional comments

The rationale for double buffering stems from the use of ranges for particle cloud initialization during resampling. Since range evaluation is lazy, the resampling algorithm would end up reading from the same particle set it is mutating, skewing the results.

Nav2 AMCL is careful about this. We were not (and that's how I realized).

This is crucial during particle resampling

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic requested a review from glpuga November 5, 2023 13:53
@hidmic hidmic self-assigned this Nov 5, 2023
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Copy link
Collaborator

@glpuga glpuga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! LGTM

Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hidmic hidmic merged commit 880505e into main Nov 6, 2023
7 checks passed
@hidmic hidmic deleted the hidmic/particle-double-buffering branch November 6, 2023 12:42
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

Successfully merging this pull request may close these issues.

None yet

3 participants