Skip to content

[RSDK-13624] Introduce support for randomized trajectory testing#192

Merged
acmorrow merged 1 commit intoviam-modules:mainfrom
acmorrow:RSDK-13624-random-trajectory-testing
Mar 18, 2026
Merged

[RSDK-13624] Introduce support for randomized trajectory testing#192
acmorrow merged 1 commit intoviam-modules:mainfrom
acmorrow:RSDK-13624-random-trajectory-testing

Conversation

@acmorrow
Copy link
Contributor

This adds support for, but does not enable, randomized testing of trajectory generation. It's found some interesting things already, but we have higher priorities. However, since we are shortly getting trajex into it's own repo, I'm trying to get anything I have on a branch out for PR so I can get it merged ahead of the split.

I'll leave some comments on what is going on in various places.

@acmorrow acmorrow requested review from JohnN193 and npmenard March 17, 2026 20:49
BOOST_CHECK_EQUAL(segments.size(), 4);
}

// Deduplication must compare against the last kept point, not the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had a bug in the trajex waypoint deduplication code! I found it sort of by accident while looking at some of the random trajectories, but I'd like to fix it here and now.

if (xt::norm_linf(waypoints[i] - waypoints[i - 1])() > tolerance) {
result.add_waypoint(waypoints[i]);
std::ranges::for_each(waypoints | std::views::drop(1), [&](const auto& waypoint) {
if (xt::norm_linf(waypoint - result.back())() > tolerance) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look carefully, you will see that the old loop only compared against the prior waypoint, not the most recently kept waypoint.

// Ensure the original last waypoint is preserved exactly as the destination.
// If it was deduplicated away, replace the last kept point with it. If the
// entire sequence collapsed to a single point, leave it as the start and let
// path creation fail with a clear error.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems like an obvious and desirable improvement, in that we really do want the path to start and end at the first and last waypoints.

return result;
}

// Expected number of reversals and colinears per random run. The per-step injection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no need to review this test infrastructure carefully unless you are interested.


// Ensure that if we run the random trajectory twice on the same seed,
// that we get deterministic outcomes.
BOOST_AUTO_TEST_CASE(random_trajectory_determinism) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only test that gets lit up as part of this PR: it generates two trajectories from the same random seed and checks that we get the same events. Trajectory generation should be fully deterministic, so this is a good check to have.

@acmorrow acmorrow merged commit 70b6235 into viam-modules:main Mar 18, 2026
7 checks passed
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.

2 participants