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

Fix integer overflow in parallel computing #3426

Merged
merged 3 commits into from
Sep 23, 2024
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/spikeinterface/core/job_tools.py
Original file line number Diff line number Diff line change
@@ -136,11 +136,8 @@ def divide_segment_into_chunks(num_frames, chunk_size):
else:
n = num_frames // chunk_size

frame_starts = np.arange(n) * chunk_size
frame_stops = frame_starts + chunk_size

frame_starts = frame_starts.tolist()
frame_stops = frame_stops.tolist()
frame_starts = [i * chunk_size for i in range(n)]
frame_stops = [(i + 1) * chunk_size for i in range(n)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works. I tend to lean toward explicit so something like


frame_stops = [frame_start + chunk_size for frame_start in frame_starts]

Just because it is clearer rather than have to multiple things out. But I think this is a good protection against overflow. Your long recordings are really finding all the overflows :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... I am surprised no one has tried to deal with long recordings before. Are there any other long-recording users?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The most I've heard of recently is closer to the 10-15 hour mark. But haven't really followed up on those. These things work until they don't. So we are trying to track them down, but they are easier to find when they break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hope that long recording is not a bad idea :)

Copy link
Member

Choose a reason for hiding this comment

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

@jiumao2 we are planning to run week long recordings. You are in good company :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to hear that! Not alone anymore :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @alejoe91 ,

Are you beginning to work with the week-long recordings? I'm not satisfied with the results from Kilosort and have run into a number of issues. Could you share any strategies or tools you're using, such as different sorters, that might be helpful?

Thank you!


if (num_frames % chunk_size) > 0:
frame_starts.append(n * chunk_size)