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

Make the trajectory prediction logic more centralized #532

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

jeremykubica
Copy link
Contributor

Predicting the trajectory's position is done manually throughout a lot of the code as trj.x + dt * trj.vx + 0.5. The 0.5 pixel offset is applied most of the time, but not everywhere (e.g. in computing the stamps). This PR centralizes the prediction logic into common.h.

At the same time it makes the 0.5 the standard default throughout the code. This should lead to long term better performance, but requires some changes to tests (for stamps). This also leads to a increase in the number of results found by the diff test.

Copy link
Member

@DinoBektesevic DinoBektesevic left a comment

Choose a reason for hiding this comment

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

Minor question regarding not using Index vs Point otherwise this is fine with me with or without the filtering or detection working differently.

Comment on lines +56 to +66
// Get pixel positions from a zero-shifted time. Centered indicates whether
// the prediction starts from the center of the pixel (which it does in the search)
inline float get_x_pos(float time, bool centered = true) const {
return centered ? (x + time * vx + 0.5f) : (x + time * vx);
}
inline float get_y_pos(float time, bool centered = true) const {
return centered ? (y + time * vy + 0.5f) : (y + time * vy);
}

inline int get_x_index(float time) const { return (int)floor(get_x_pos(time, true)); }
inline int get_y_index(float time) const { return (int)floor(get_y_pos(time, true)); }
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this deserves a get_pos that returns an Index or an Point and then snaps them to the respective grids. (For some reason this feels like it was done at one point but there are no traces of this in the code at all?)

On the GPU we have a separate predict_index anyhow and we should use our own inventions for consistency at least.

@jeremykubica jeremykubica merged commit ae4b2e4 into main Apr 16, 2024
2 checks passed
@jeremykubica jeremykubica deleted the prediction_consistency branch April 16, 2024 15:37
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