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

[ENH] Much faster mahal cleaning #982

Merged
merged 6 commits into from
Aug 22, 2023
Merged

Conversation

36000
Copy link
Collaborator

@36000 36000 commented Aug 10, 2023

It my case, it reduced the clean time from around 15 minutes to around 4 seconds (225x speedup).
We do this by using numpy functions instead of for loops. In _fixes for now, but can be moved to dipy later.

@arokem
Copy link
Collaborator

arokem commented Aug 10, 2023

This is awesome! Probably also more memory efficient (?). Can you write a test that compares it against the slow implementation? We should definitely push this upstream to DIPY.

@36000
Copy link
Collaborator Author

36000 commented Aug 10, 2023

Good call. I was incorrectly oversimplifying the covariance calculation, but now I fixed that. It is now around 8 seconds on my test datasets, so still a significant improvement on the dipy code, and with the same result! It is probably more memory efficient, or at least as memory efificent.

@36000
Copy link
Collaborator Author

36000 commented Aug 11, 2023

TODO: rename and conform to dipy api and system (already 90% of the way there), then just import from fixes so there is no change to the clean_bundles code

Copy link
Collaborator

@arokem arokem left a comment

Choose a reason for hiding this comment

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

Looks great now!

@36000 36000 merged commit 6e3625f into yeatmanlab:master Aug 22, 2023
8 checks passed
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