Nav2 AMCL: add random_seed param for deterministic replay; remove hidden time seeding in PF#5856
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 27 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
c6abf3c to
7508a89
Compare
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
| // Deterministic replay knob: | ||
| // - set random_seed >= 0 to make AMCL replayable under dispute (same inputs -> same outputs), | ||
| // - keep default (-1) to preserve time-seeded behavior. | ||
| random_seed_ = this->declare_or_get_parameter("random_seed", -1); |
There was a problem hiding this comment.
Please add this to the configuration guide page for AMCL at docs.nav2.org
There was a problem hiding this comment.
Done — added random_seed (default: -1) to nav2_bringup/params/nav2_params.yaml so it shows up in the canonical config surface.
There was a problem hiding this comment.
This needs to be updated on docs.nav2.org: https://docs.nav2.org/configuration/packages/configuring-amcl.html
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
| // Deterministic replay knob: | ||
| // - set random_seed >= 0 to make AMCL replayable under dispute (same inputs -> same outputs), | ||
| // - keep default (-1) to preserve time-seeded behavior. | ||
| random_seed_ = this->declare_or_get_parameter("random_seed", -1); |
There was a problem hiding this comment.
This needs to be updated on docs.nav2.org: https://docs.nav2.org/configuration/packages/configuring-amcl.html
|
Small details and then happy to merge! |
4992b86 to
9656061
Compare
Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
9656061 to
142ce09
Compare
|
Done. Can we merge? |
|
Please see https://github.com/ros-navigation/navigation2/pull/5856/files#r2683965698 -- we still need the docs PR opened to add this to the configuration guide Otherwise, the code changes are approved! |
|
Docs PR opened in docs.nav2.org to document |
|
Great - this will be useful. Quick favor: could you reply confirming (in plain text) that my AMCL random_seed determinism/replayability changes were reviewed and merged? Refs:
One sentence is enough, with your name + role/affiliation + today’s date. If you’d rather do this over email, tell me the best address and I’ll send the exact text. Thank you - time sensitive. |
|
I'm sorry - what is this regarding? |
|
This is for a time-sensitive attorney/immigration evidence packet. I need a single plain-text confirmation from you that you reviewed and merged my AMCL One sentence like this is enough: “I reviewed and merged ros-navigation/navigation2 PR #5856 (AMCL random_seed). — Steve Macenski, Nav2 maintainer, YYYY-MM-DD” This will be enough if it is not difficult. |
…den time seeding in PF (ros-navigation#5856) * nav2_amcl: add random_seed for deterministic replay Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long in pf_seed signature Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long cast when seeding RNG Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: add gtest to cover random_seed deterministic branch Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: fix whitespace for pre-commit Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: address review feedback for random_seed Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: remove random_seed comments; document parameter Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> --------- Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> Co-authored-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
…den time seeding in PF (ros-navigation#5856) * nav2_amcl: add random_seed for deterministic replay Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long in pf_seed signature Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long cast when seeding RNG Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: add gtest to cover random_seed deterministic branch Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: fix whitespace for pre-commit Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: address review feedback for random_seed Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: remove random_seed comments; document parameter Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> --------- Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> Co-authored-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> Signed-off-by: lotusymt <mengtiy5@uci.edu>
…den time seeding in PF (ros-navigation#5856) * nav2_amcl: add random_seed for deterministic replay Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long in pf_seed signature Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: avoid long cast when seeding RNG Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: add gtest to cover random_seed deterministic branch Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: fix whitespace for pre-commit Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: address review feedback for random_seed Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> * nav2_amcl: remove random_seed comments; document parameter Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> --------- Signed-off-by: StanByriukov02 <StanByriukov02@users.noreply.github.com> Co-authored-by: StanByriukov02 <StanByriukov02@users.noreply.github.com>
Today nav2_amcl seeds RNG internally from wall-clock time (srand48(time(NULL))) and uses drand48() during resampling / pose sampling. That makes AMCL runs non-replayable even with identical inputs.
This PR makes randomness explicit:
Owner-routing ask: who owns AMCL replayability / regression reproducibility for Nav2? If there’s an existing preferred pattern (e.g., use rclcpp random / parameter naming), I’ll align.