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.

Sign up for GitHub

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

Jump to bottom

Prefer forward critic optimization #86

Merged
merged 3 commits into from
Jul 21, 2022
Merged

Prefer forward critic optimization #86

merged 3 commits into from
Jul 21, 2022

Conversation

artofnothingness
Copy link
Owner

No description provided.

SteveMacenski
SteveMacenski reviewed Jul 21, 2022
View reviewed changes
CMakeLists.txt
@@ -4,6 +4,10 @@ project(mppic)
add_definitions(-DXTENSOR_ENABLE_XSIMD)
add_definitions(-DXTENSOR_USE_XSIMD)

set(XTENSOR_USE_TBB 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't 1 be enabling? Does this change anything?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure, but i would prefer to keep things more one threaded till we reduce jitter problem. One thread - more predictable behavior

@SteveMacenski
Copy link
Collaborator

SteveMacenski commented Jul 21, 2022 •
edited
Loading

@artofnothingness I pushed a commit here that I think you'll approve of. I changed it use the same maximum + sum as before, and scaled the velocities by the model dt to give positions I added to the critics data (that would generally be useful for some critics to know). This allows this critic to maintain the same order of magnitude from before, representing reversing distance, so we don't need to tune this critic again (or as much).

The immediate on sum is documented in xtensor to really speed things up.

I tested this and happy with the performance. I was pretty happy with it too in your initial commits, but I wanted to try to mimic historical behavior / tuned parameters as much as possible. The CPU change between them is negligible. Now that this critic removes 2 trig functions, I see the spikes less frequently cause issues with the loop rate. Spikes into the ~20s of ms rather than the ~30s of ms.

I'm happy to merge this if you are.

@SteveMacenski SteveMacenski mentioned this pull request Jul 21, 2022
Closed
@artofnothingness
Copy link
Owner Author

We can merge

@artofnothingness artofnothingness changed the title ini Prefer forward critic optimization Jul 21, 2022
@SteveMacenski SteveMacenski merged commit bba975b into develop Jul 21, 2022
@SteveMacenski SteveMacenski deleted the f/perfer-forward-optimize branch July 21, 2022 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SteveMacenski SteveMacenski SteveMacenski left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@artofnothingness @SteveMacenski