Skip to content

Rework generation dataflow to avoid mutating parameters #1257

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shueja
Copy link
Collaborator

@shueja shueja commented May 16, 2025

Previously:

  • Mutate parameters to adjust headings (by changing the original heading expression) and identify initial guess points (by changing a nonserialized helper field on Waypoint)
  • Take a snapshot of the mutated parameters and use that to set up the SwerveTrajectoryGenerator.
  • During TrajectoryGenerator setup, guess the control interval counts and handle them separately from the snapshot
  • After generation, guess the control interval counts again and update both the snapshot and the params section with new counts.

This results in bugs where the generator wipes away the expressions in waypoint headings. This was identified as a CLI bug, but it is a core bug that's masked by the way the frontend handles generation results.

New behavior:

  • Take a snapshot immediately of the params, never change the params again.
  • Mutate clones of the snapshot to adjust headings, identify initial guess points, and store the control interval guess in the snapshot.waypoints[n].intervals fields.
    • The functions for these now take a snapshot instead of taking the whole TrajectoryFile and modifying its .params
  • Set up the TrajectoryGenerator with the modified snapshot.
  • After generation, do not update the params section of the trajectory with the updated control interval counts. The snapshot section is the new source of truth for how waypoints correspond to the samples array.

Also this PR has a new docs page explaining what parts of the TrajectoryFile change upon generation.

GenerationEquivalent

Since the end condition of generation no longer has total equivalence between the params and snapshot, the equality checks for upToDate have been replaced with a custom trait GenerationEquivalent. This helps better capture aspects of parameters that are conditionally irrelevant. The special cases for equivalence where equality would not hold are currently as follows.

  • Constraint: differences in data are ignored if both constraints are not enabled.
  • Waypoint: differences in intervals are ignored if both override_intervals are false.

@shueja shueja added the component: backend Rust/Tauri backend label May 16, 2025
@github-actions github-actions bot added the component: documentation Improvements or additions to documentation label May 16, 2025
@shueja
Copy link
Collaborator Author

shueja commented May 17, 2025

I tested with the src-core integration tests that:

  • on main, a param.waypoint.heading of {"0 deg", 0.0} was output as {"0 rad", 0.0}, which matches the buggy behavior this PR tries to fix.
  • on this branch, the same 0 deg parameter is output unchanged, as expected.
  • on this branch, a manually-set param.waypoint.intervals of 86, without override_intervals on, was output unchanged, but the snapshot.waypoint.intervals was 27, as was the correct heuristic value.

This demonstrates that the parameter section is no longer getting modified, but the snapshot section does still communicate the correct interval counts.

Copy link
Member

Choose a reason for hiding this comment

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

The implementation should link to this in relevant places so it's more likely to get updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: backend Rust/Tauri backend component: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants