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

improve performance #32

Open
stonylohr opened this issue Jan 24, 2021 · 2 comments
Open

improve performance #32

stonylohr opened this issue Jan 24, 2021 · 2 comments

Comments

@stonylohr
Copy link
Contributor

stonylohr commented Jan 24, 2021

Some avenues to consider, in roughly chronological order of observation, include:

  1. Issues replace get_epsilon with f64 EPSILON #1, replace get_min_val with f64 MIN_POSITIVE #2, reduce use of variable shadowing (geomath) #3, replace geomath::cbrt with f64::cbrt #4, replace geomath::fmod with direct calls to % operator #5, consider simplifying polyval #6, consider changes for sincosd #7, changes to consider for atan2d #8, consider changes to ang_round #9, avoid passing GEODESIC_ORDER as argument #10, and favor type usize for variables that are primarily an array index #12 for geomath.
  2. Modify geomath::norm to pass values by mutable reference and update them in place rather than returning a separate value.
  3. Negate geodesic._gen_inverse clam12 on the same line where it's assigned, and make it non-mut.
  4. Explore use of f64.sin_cos() over f64.sin() followed by f64.cos().
  5. Explore use of mem::swap() and relatives in place of things like {let c=a; a=b; b=c;}
  6. Look for places that assign the same calculated value more than once in a row, like:
    M12 = sig12.cos();
    M21 = sig12.cos(); // instead use M21 = M12;
    
  7. Explore reducing scratch array allocations, through some combination of (a) more reuse and (b) larger allocations followed by slicing.
  8. Review choice of declared types, to reduce need for casting.
  9. Make more use of operation-assign operators, like |=.
  10. For optional inputs, standardize (at least for each individual case) on either None or NAN as the "no value" option, rather than using a 2-step translation process (e.g. None to NAN to calculated default).

None is likely to make much impact individually, but they might add up to something together.

I'll continue adding items to this list as I explore issues with returned values, until we get around to actually trying to make performance improvements.

@valarauca
Copy link
Contributor

Actually been attacking these problems on my own.

I Currently have the rust implementation at ~7.06% faster than the reference C implementation. I hope to open a PR by sunday. If the project is interested in these changes.

@stonylohr
Copy link
Contributor Author

stonylohr commented Jan 19, 2024 via email

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

No branches or pull requests

2 participants