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

speedup in _where #630

Closed
wants to merge 2 commits into from
Closed

Conversation

llllllllll
Copy link

I saw my code spending a lot of time in the coords = [p.nrow ...] comprehension, this change should speed up the creation of the coordinates array dramatically. The monotonic check removal is because it took a long time to create the array to do the check, it seems to be fine to just skip it because the savings are lost by allocating and doing the elementwise compare.

The idea of the change is to do the iteration directly in cython to produce a coordinate array.

@scopatz
Copy link
Member

scopatz commented May 15, 2017

This seems reasonable to me. Any idea what kind of speedup is seen?

@llllllllll
Copy link
Author

Not yet, I haven't tested too much. I wanted to make sure this seemed like a reasonable change before investing too much time. I will try to test this with the different __next__ implementations with different sized tables.

@FrancescAlted
Copy link
Member

The problem that I see with this approach is that you are malloc'ing a potentially big out array (as long as the length of the table) for the coordinates, and this might require a lot of memory. I agree that as Linux does overcommit, this is rarely a problem, but this does not apply to Mac OSX and Windows. Maybe you can start with a small out array and enlarge it as needed?

@jsancho-gpl jsancho-gpl changed the base branch from develop to master June 12, 2018 14:01
@llllllllll llllllllll closed this Sep 13, 2018
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