-
Notifications
You must be signed in to change notification settings - Fork 190
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
Speed up searchsorted calls #2000
Conversation
for more information, see https://pre-commit.ci
Thanks Pierre! Can you check why tests are failing? |
Co-authored-by: Zach McKenzie <[email protected]>
import numpy as np
x = np.arange(int(1e8))
first_frame = 500_000
chunk_size = 100_000
last_frame = first_frame + chunk_size
# First case: Two individual searches
def search_individual():
left1 = np.searchsorted(x, first_frame)
right1 = np.searchsorted(x, last_frame)
return left1, right1
# Second case: A single search with both values
def search_together():
left2, right2 = np.searchsorted(x, [first_frame, last_frame])
return left2, right2
# Use the timeit magic command to compare
print("First case:")
%timeit search_individual()
print("\nSecond case:")
%timeit search_together() Output in my machine:
|
5 % improvement for every chunk acumulates. This looks good, @yger . Not relevant in this PR but I wish we used better naming for the left and right bounds than They are comfortable for the person that knows the code (like algebraic expressions are comfortable to operate in a mathematical theory that you already dominate) but are difficult, terse and non-obvious for someone who encounters the expression for the first time. |
I thought it would have been larger, but a gain is a gain :-) |
This is ready to be merged @alejoe91 @samuelgarcia , I only handled the cases where the searchsorted were of the same type (left or right) |
Quite a lot everywhere in spikeinterface, searchsorted is called twice and thus a speedup can be gained by making a single call