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

MAINT: minor cython cleanup in align/vector_fields.pyx #1049

Merged
merged 4 commits into from
Sep 11, 2016

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented May 13, 2016

These are minor changes to remove python interaction from within some of the loops present in vector_fields.pyx.

It is easiest to verify the reduction in Python interaction via observing the reduction in yellow lines in the annotated HTML files generated by cython -a vector_fields.pyx in this PR vs master.

The following four functions receive approximately 3 orders of magnitude speedup by avoiding calls to np.cos, np.arctan2 and/or np.sqrt:
create_circle, create_sphere, create_harmonic_fields_2d, create_harmonic_fields_3d

For the rest, there is negligable difference in performance. I have also looked at enabling parallel processing via prange in some of these functions and plan to submit that as a separate future PR. (In the multithreaded case, the change of _interpolate_vector_2d and _interpolate_vector_3d to use floating* out instead of floating[:] out makes a huge difference to some of the routines that were passing a view of a memoryview which was causing things to actually become slower if multithreaded)

@grlee77 grlee77 changed the title MAINT: minor cython cleanup in vfields_parallel.pyx MAINT: minor cython cleanup in align/vector_fields.pyx May 14, 2016
@omarocegueda
Copy link
Contributor

This looks good to me (sorry for the delay, I think this should have been merged immediately!)

@Garyfallidis
Copy link
Contributor

Good job @grlee77!

@Garyfallidis Garyfallidis merged commit 2fc0485 into dipy:master Sep 11, 2016
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.

3 participants