Skip to content

Conversation

@nycrat
Copy link
Member

@nycrat nycrat commented Dec 1, 2025

Description

Fixes bug where ball velocity is inaccurately being calculated/estimated. The bug is caused by the following logic:

  • estimateBallStateFromBuffer calls calculateLineOfBestFit to get the trajectory of the ball
  • estimateBallVelocity is optionally passed this trajectory, setting the current + previous positions (used to calculate velocity) based on the trajectory
  • However, the calculateLineOfBest function does two linear regressions, one with x vs y and one with y vs x (to account for vertical lines) and selects the one with least error
  • In the calculateLinearRegression function, the error is calculated as relative error, which causes an issue when the x or y coordinate is close to zero, since it divides by it

The simple fix is just to use absolute error, since relative error for linear regression in this context of coordinates makes no sense, since we expect similar error for the estimated trajectory of the ball no matter where it is on the field.

Testing Done

Ran tests for ball_filter and passes. Also ran the original test that caused this bug and it is fixed (proper velocity being reported)

Resolved Issues

fixes #3527

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

  • Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
  • Remove all commented out code
  • Remove extra print statements: for example, those just used for testing
  • Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

GrayHoang
GrayHoang previously approved these changes Dec 1, 2025
Copy link
Contributor

@GrayHoang GrayHoang left a comment

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor

@Andrewyx Andrewyx left a comment

Choose a reason for hiding this comment

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

This makes sense to me, but can you update ball_filter.h to include the fact it returns the value of the error itself now? Additionally, if possible, it would be helpful to note if that value is in terms of the MSE just so that future reference to this function is clearer as to how the regression error is being generated. Otherwise, nice work!

@nycrat nycrat requested review from Andrewyx and GrayHoang December 4, 2025 05:20
GrayHoang
GrayHoang approved these changes Dec 4, 2025
Copy link
Contributor

@StarrryNight StarrryNight left a comment

Choose a reason for hiding this comment

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

Nice!

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.

Inaccurate velocity in world state message being passed to Validation

4 participants