[cboulay]

For a multi-segment .nev, BlackrockRawIO clipped spikes and events to the segment's time window: the bounds were float seconds converted back to ticks, then compared with np.searchsorted (which compares uint64 with a Python int as float64). Float64 seconds at 1.75e9 (PTP files, 1 ns resolution) are only precise to about 240 ns, so items at a segment's edge were kept or dropped by rounding. spike_count/event_count used the same window and could disagree with get_spike_timestamps. Non-PTP files (30 kHz ticks) are not affected.

On the NEV of blackrock_ptp_with_missing_samples opened alone (4 segments), 4 of 999 spikes were lost, with and without explicit bounds. Converting explicit bounds to exact integer ticks does not fix it (5 lost), because the rounding is in the float seconds the caller passes.

Changes:

• Without t_start/t_stop, items are selected by segment id alone (the getters already did), and spike_count/event_count do the same.
• With explicit bounds, timestamps are compared as float64 seconds, the same values neo reports, so a time taken from neo (e.g. segment_t_start) selects the item it came from.
• t_stop stays exclusive. This is now documented, with the advice to pass t_stop + 1e-5 to include an item at t_stop (the old +1 tick in the default path was a no-op at 1 ns).

The new test checks every unit of every segment of that file against the segment ids, with and without bounds, and fails on the old code.

Found while working on Blackrock reading with Colin McNabb at Blackrock Neurotech.

[h-mayorquin]

This looks good. Using the segment ids that were already computed instead of recalculating the membership from the time windows is better as it states the identity directly. This is what I did in #1880.

The fixes seem OK to me but my only qualms are about testing. My requests:

• Add blackrock/segment/ResetCorrect/reset to entities_to_test. It has a real clock reset, so the split stays whatever we decide in #1937 (two segments now, two blocks if a reset starts a new block). The shared checks already compare spike_count with the timestamps returned for each segment (read_spike_times in rawio_compliance.py), and on this file they fail on master (nb_spike 40 != 188) and pass here. So the count fix needs no new test code, and the other shared checks run on a multi-segment Blackrock file for the first time.
• For the spikes lost at the segment edges a specific test is still needed because it takes the PTP file read without its .nsx. I think it could be simpler and use only the public API: check that the spikes returned over every block, segment and unit add up to the 999 in the file (master returns 995). Something like:

def test_nev_spikes_at_segment_edges_ptp(self):
    reader = BlackrockRawIO(
        filename=self.get_local_path("blackrock/blackrock_ptp_with_missing_samples/Hub1-NWBtestfile_neural_wspikes"),
        nsx_to_load=[],
    )
    reader.parse_header()
    returned = 0
    for block_index in range(reader.block_count()):
        for seg_index in range(reader.segment_count(block_index)):
            for unit_index in range(reader.spike_channels_count()):
                returned += reader.get_spike_timestamps(block_index, seg_index, unit_index, None, None).size
    self.assertEqual(returned, 999)

I think this way of testing is better because:

• Adding the file to entities_to_test reuses the checks that every reader already runs, so the multi-segment case gets all of them and not only the count.
• The specific test uses only the public API, so it does not break if nev_data or internal_unit_ids change in a refactor.
• It checks what the shared checks cannot, that every spike in the file comes back somewhere. That has to hold for any segmentation, so it keeps working whatever we decide in #1937.
• The expected value comes from the file itself, the number of spikes it holds, so it does not depend on how the reader assigns the segments.

The current test failures come from the gin 403 and not from this PR. #1938 should fix them.

[h-mayorquin]

This is ready, thanks for the changes on the testing.