Saving a multi-segment recording fails when only some segments have a t_start

Open Beginner friendly
  • Is something wrong?
#4,914 1 comment 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
numpy, python
Domain
backend, data

Research direction

Start with the three constructors named in the issue: NumpyRecording and SharedMemoryRecording in src/spikeinterface/core/numpyextractors.py, and BinaryRecordingExtractor in src/spikeinterface/core/binaryrecordingextractor.py. Read how each handles get_segment_t_starts, then run the reproduction and the named recording and binary extractor tests. Done means binary and memory saves preserve the mixed start times and traces without leaked shared memory.

Written by the indexing model from the issue text.

Description

core

get_segment_t_starts returns a mixed list such as [None, 100.0] when only some segments have a start time. Three constructors then call float() on every entry. This happens with append_recordings([rec_without_t_start, rec_with_t_start]), and more generally when shift_times has been applied to only some segments.

Reproduction

main at 9ef464e66, Linux (WSL Ubuntu 24.04), Python 3.12.3, numpy 2.5.4, zarr 3.4.2:

import spikeinterface.core as sc

a = sc.generate_recording(num_channels=2, sampling_frequency=1000.0, durations=[3.0], seed=0)
b = sc.generate_recording(num_channels=2, sampling_frequency=1000.0, durations=[2.0], seed=1)
b.shift_times(100.0)
rec = sc.append_recordings([a, b])
print(rec.get_segment_t_starts(), [rec.get_start_time(i) for i in range(2)])

for kwargs in [
    dict(format="binary", folder="rec_binary"),
    dict(format="memory", shared_memory=False),
    dict(format="memory", shared_memory=True),
    dict(format="zarr", folder="rec.zarr"),
]:
    try:
        saved = rec.save(**kwargs)
        print(kwargs["format"], "OK", [saved.get_start_time(i) for i in range(2)])
    except TypeError as ex:
        print(kwargs["format"], f"TypeError: {ex}")

[None, 100.0] [0.0, 100.0]
binary TypeError: float() argument must be a string or a real number, not 'NoneType'
memory TypeError: float() argument must be a string or a real number, not 'NoneType'
memory TypeError: float() argument must be a string or a real number, not 'NoneType'
zarr OK [0.0, array(100.)]

Three details make this worse:

  • Binary: the exception comes after all traces are written. In the example, rec_binary contains traces_cached_seg0.raw and traces_cached_seg1.raw, but neither binary.json nor si_folder.json. For a long recording, this means that hours of writing end in a folder that cannot be loaded.
  • Shared memory: the exception comes before self.main_shm_owner is set, so __del__ raises AttributeError: 'SharedMemoryRecording' object has no attribute 'main_shm_owner', and the segments are not unlinked. The script ends with resource_tracker: There appear to be 4 leaked shared_memory objects to clean up at shutdown.
  • Zarr: saving works, but get_start_time(1) returns a 0-d array(100.) instead of a float.
Cause

get_segment_t_starts only returns None when all segments have no start time. The conversion is in NumpyRecording, SharedMemoryRecording and BinaryRecordingExtractor. In SharedMemoryRecording, main_shm_owner is set after this line, and __del__ reads it.

Possible fix

In the three constructors, keep None:

t_starts = [None if t_start is None else float(t_start) for t_start in t_starts]

With this, the binary and both memory saves work, with start times [0.0, 100.0] and identical traces, and the script no longer reports leaked segments. test_basesorting.py, test_baserecording.py, test_binaryfolder.py, test_binaryrecordingextractor.py and test_numpy_extractors.py still pass (33 tests).

Two separate points would remain: setting main_shm_owner before anything that can fail, so that __del__ is safe after another error, and the 0-d array from zarr. I have not changed them.

I found no report of this case. Nearby: #2515 and #3120 (t_starts lost by save_to_memory) and #3940 (shift_times when t_start is None). I can open a PR with this change and a round-trip test.

Dominant language
Python
Stars
870
Forks
284
Avg merge
3d 1h
Merged PRs (30d)
41

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from SpikeInterface/spikeinterface

All issues in SpikeInterface/spikeinterface

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.