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

[Utils] Streamline utils_saturate_vector_2d() #460

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kubark42
Copy link
Contributor

There was no need to do the sqrt() unless the absolute magnitude is exceeded.

Passes unit tests but shouldn't be merged until #459 is merged.

@vedderb
Copy link
Owner

vedderb commented Mar 25, 2022

It looks like the worst case, when saturation is needed, will still take the same amount of time. Also, sqrtf is quite fast on the FPU.

For optimizations like this one it would be good to set up a way to profile how much difference they make. Maybe this can be done by running a test on the STM32 with some build option and plot the result in VESC Tool.

There was no need to do the `sqrt()` unless the absolute magnitude is
exceeded.
@kubark42 kubark42 force-pushed the util_saturate_vector_optimize branch from 3e78449 to baf92a6 Compare March 26, 2022 00:11
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