MergeUnitsSorting: merged units come back empty from frame_slice and split_sorting

[Arthur031221]

MergeUnitsSortingSegment.get_unit_spike_train computes the end of the slice by searching for start_frame instead of end_frame. The slice is therefore the entries equal to start_frame, at most one spike, so reading a merged unit through a frame window comes back empty unless start_frame happens to land on a spike.

spikeinterface/src/spikeinterface/curation/mergeunitssorting.py

Lines 159 to 167 in d365714

	if start_frame is not None:
	start_i = np.searchsorted(spike_times, start_frame, side="left")
	else:
	start_i = 0
	if end_frame is not None:
	end_i = np.searchsorted(spike_times, start_frame, side="right")
	else:
	end_i = len(spike_times)
	return spike_times[start_i:end_i]

The defect is in the segment path. Calling get_unit_spike_train(unit, start_frame=..., end_frame=...) on the merged sorting with the default use_cache=True resolves the window against a cached spike vector, and gave the right answer for every window I tried. The window has to reach the segment for this to show, which is what frame_slice, time_slice, split_sorting and use_cache=False all do. Units that were not merged are handed to the parent segment on the else branch and are unaffected either way, so a sliced sorting looks healthy apart from the merged units being empty.

Reproduction

import numpy as np
from spikeinterface.core import NumpySorting
from spikeinterface.curation import merge_units_sorting

trains = {
    0: np.array([100, 300, 500]),
    1: np.array([200, 400, 600]),
    2: np.array([150, 350]),
}
sorting = NumpySorting.from_unit_dict(trains, sampling_frequency=30000.0)
merged = merge_units_sorting(sorting, [[0, 1]], new_unit_ids=[3])

print("no window            ", merged.get_unit_spike_train(3))

sliced = merged.frame_slice(start_frame=0, end_frame=1000)
print("frame_slice, unit 3  ", sliced.get_unit_spike_train(3))
print("frame_slice, unit 2  ", sliced.get_unit_spike_train(2))

t_sliced = merged.time_slice(start_time=0.0, end_time=1000 / 30000.0)
print("time_slice, unit 3   ", t_sliced.get_unit_spike_train(3))
print("time_slice, unit 2   ", t_sliced.get_unit_spike_train(2))

no window             [100 200 300 400 500 600]
frame_slice, unit 3   []
frame_slice, unit 2   [150 350]
time_slice, unit 3    []
time_slice, unit 2    [150 350]

The window covers every spike in the sorting.

Through split_sorting

split_sorting() slices with frame_slice internally, so splitting a concatenated recording after a merge loses the merged units:

import numpy as np
from spikeinterface.core import (
    NumpySorting, generate_recording, concatenate_recordings, split_sorting,
)
from spikeinterface.curation import merge_units_sorting

fs = 30000.0
recs = [generate_recording(num_channels=2, durations=[1000 / fs], sampling_frequency=fs, seed=0)
        for _ in range(2)]
cat = concatenate_recordings(recs)

trains = {0: np.array([100, 300]), 1: np.array([200, 400]), 2: np.array([150, 1200])}
sorting = NumpySorting.from_unit_dict([trains], sampling_frequency=fs)
sorting.register_recording(cat)
merged = merge_units_sorting(sorting, [[0, 1]], new_unit_ids=[3])
merged.register_recording(cat)

sp = split_sorting(merged, recording_or_recording_list=recs)
for seg in range(sp.get_num_segments()):
    print(f"segment {seg} unit 3 (merged)   :", sp.get_unit_spike_train(3, segment_index=seg))
    print(f"segment {seg} unit 2 (untouched):", sp.get_unit_spike_train(2, segment_index=seg))

segment 0 unit 3 (merged)   : []
segment 0 unit 2 (untouched): [150]
segment 1 unit 3 (merged)   : []
segment 1 unit 2 (untouched): [200]

Unit 3 holds [100 200 300 400], all inside the first 1000 samples, so segment 0 should carry those four and segment 1 should be empty. Both are empty.

This is the symptom in #2555, closed as stale in February 2025. I can't claim that reporter's script still reproduces: they reached the merge through apply_sortingview_curation, which now routes through apply_curation and apply_merges_to_sorting and does not build a MergeUnitsSorting. The other route suggested in that thread does still go through this slice. select_segment_sorting(split_sorting(merged, ...), segment_indices=0) returns [] for the merged unit and [150] for the untouched one on d365714a4.

The change I tested

-                end_i = np.searchsorted(spike_times, start_frame, side="right")
+                end_i = np.searchsorted(spike_times, end_frame, side="left")

Two things are wrong on that line and the variable is only the first. Keeping side="right" while fixing the variable makes the segment path disagree with the cached path of BaseSorting.get_unit_spike_train, which searches end_frame with the numpy default of side="left" at core/basesorting.py:215. Same object, same call, two answers depending on use_cache. Spike times [100 200 300 400 500 600] on the merged unit, window start_frame=200, end_frame=500:

line 164	use_cache=False	use_cache=True
start_frame, side="right" (main)	[200]	[200 300 400]
end_frame, side="right"	[200 300 400 500]	[200 300 400]
end_frame, side="left"	[200 300 400]	[200 300 400]

side="left" is what FrameSliceSorting already documents at core/frameslicesorting.py:27-29, "As for usual python slicing, the end frame is excluded", and it matches SpikeVectorSortingSegment.get_unit_spike_train at core/basesorting.py:1287, which is the segment class NumpySorting itself uses.

Reaching the segment with use_cache=False, start_frame=None and an end_frame set raises TypeError: '<' not supported between instances of 'NoneType' and 'NoneType' on main, because None is handed to np.searchsorted. The branch that handles end_frame is None is already correct and is untouched, so the TypeError goes away with the same one line change.

With that one line changed and two regression tests added to curation/tests/test_curationsorting.py, on Linux, Python 3.12.3, numpy 2.5.2, same tree before and after:

• pytest src/spikeinterface/curation/tests: 74 passed, 3 skipped with the fix, and 72 passed, 2 failed, 3 skipped with the source line put back, the two failures being the new tests.
• pytest src/spikeinterface/core: 325 passed, 3 skipped both ways.

Two neighbours I left out

curation/remove_duplicated_spikes.py:89 and curation/splitunitsorting.py:147 search end_frame with side="right", so both include a spike sitting exactly on end_frame while the cached path excludes it. On d365714a4, a unit with spikes [100 500 900]:

RemoveDuplicatedSpikesSorting, unit 0
  end_frame=899: segment=[100 500]      cached=[100 500]      agree=yes
  end_frame=900: segment=[100 500 900]  cached=[100 500]      agree=NO
  end_frame=901: segment=[100 500 900]  cached=[100 500 900]  agree=yes

SplitUnitSorting, child unit holding [900]
  end_frame=899: segment=[]             cached=[]             agree=yes
  end_frame=900: segment=[900]          cached=[]             agree=NO
  end_frame=901: segment=[900]          cached=[900]          agree=yes

#1772 went the same way on the same question elsewhere: it changed side="right" to side="left" on the Phy extractor's end bound with the comment # Exclude end frame, and was merged in July 2023. By that argument these two are the same bug. I separated them only because their current output is a usable spike train wrong by one spike on the boundary, while the merge case returns nothing at all, so the two carry different regression risk. Your call whether they belong in the same fix.

Versions

main at d365714a4b48b1dfb67c2d380529f402dd9e54e1, Python 3.12.3, numpy 2.5.2, Linux. It is in the 0.104.8 sdist on PyPI at src/spikeinterface/curation/mergeunitssorting.py:162, so it is in the current release and not only on main.

This is not a recent regression. fb030c2 (February 2023) only changed the array operand, and the start_frame end bound is already in its pre-image. The same start_frame end bound, over a different array, is in spikeinterface/core/mergesorting.py before the May 2022 file move in 733d1ad.

MergeUnitsSorting is listed under Deprecated in doc/api.rst, and on #3459 samuelgarcia wrote that CurationSorting "will depracted one day and totally unmaintened. You should not use it." If the answer here is that these classes are going away, that closes the issue and I have no argument with it. Two things pushed me to file anyway. The failure is silent, so you get an empty spike train rather than an error, and nothing warns at runtime. And the class is not reached only from the deprecated surface: compute_merge_unit_groups, which is the named replacement for get_potential_auto_merge and is documented at doc/api.rst:405, above the Deprecated heading at 432, builds one at auto_merge.py:1155 on its quality_score step, which four of the five presets include and the default is one of them. benchmark/benchmark_motion_interpolation.py:80 builds one too. Neither call site passes a frame window, so I am not offering them as more breakage, only as the reason the class is still load bearing.