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 phase shift memory efficiency #2946

Merged
merged 8 commits into from
Jun 5, 2024

Conversation

h-mayorquin
Copy link
Collaborator

@h-mayorquin h-mayorquin commented May 31, 2024

Been testing how much memory does phase shift uses.

Usings scipy instead of numpy I got from

18.3 GiB allocation to 10.3 GiB allocation.

Command line: /home/heberto/miniconda3/envs/neuroconv_env/bin/memray run --native --output /home/heberto/development/spikeinterface/build/memray-dev_profile.bin --force --follow-fork /home/heberto/development/spikeinterface/build/dev_profile.py
Start time: 2024-05-31 05:32:40.122000
End time: 2024-05-31 05:33:45.693000
Total number of allocations: 59354
Total number of frames seen: 2905
Peak memory usage: 18.3 GiB
Python allocator: pymalloc

to:

Command line: /home/heberto/miniconda3/envs/neuroconv_env/bin/memray run --native --output /home/heberto/development/spikeinterface/build/memray-dev.bin --force --follow-fork /home/heberto/development/spikeinterface/build/dev.py
Start time: 2024-05-31 05:30:53.499000
End time: 2024-05-31 05:32:06.583000
Total number of allocations: 64270
Total number of frames seen: 3711
Peak memory usage: 10.3 GiB
Python allocator: pymalloc

for the following script:

from spikeinterface.core import generate_recording_by_size
from spikeinterface.preprocessing import phase_shift
import numpy as np

recording = generate_recording_by_size(full_traces_size_GiB=1.0)
phase_shift_array = np.random.rand(recording.get_num_channels())
phase_shift_recording = phase_shift(recording, inter_sample_shift=phase_shift_array)
phase_shift_recording.get_traces()

I tested this against @alejoe91 data for the hackaton to see that the behavior is not changed and profiled the two (the chunk is medium size):

image

Following a suggestion by @DradeAW I also tested the speed differences in a smaller chunk:

image

My reading is that the timing differences are dwarfed by the memory improvements.

We can leave the keyword (probably better named use_scipy_fft) or not, probably better not to leak such details.

@h-mayorquin h-mayorquin added preprocessing Related to preprocessing module performance Performance issues/improvements labels May 31, 2024
@h-mayorquin h-mayorquin self-assigned this May 31, 2024
@alejoe91 alejoe91 added the hackathon-24 Contributions during the SpikeInterface Hackathon May 24 label Jun 1, 2024
@samuelgarcia samuelgarcia merged commit 3a333c4 into SpikeInterface:main Jun 5, 2024
11 checks passed
@h-mayorquin h-mayorquin deleted the optimize_phase_shift branch June 5, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hackathon-24 Contributions during the SpikeInterface Hackathon May 24 performance Performance issues/improvements preprocessing Related to preprocessing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants