Skip to content

BiocamRawIO: per-channel loop over already-loaded data makes BRW4 reads ~18x slower than necessary #1892

Description

@bmasivyer

Affects: current neo/rawio/biocamrawio.py (checked against the version
shipped with SpikeInterface 0.104.8).
Severity: performance only — output is correct.


Summary

BiocamRawIO._get_analogsignal_chunk handles BRW4 files whose raw dataset is a
flat 1-D array by looping over channels:

# neo/rawio/biocamrawio.py:187-191
sig_chunk = np.zeros((i_stop - i_start, len(channel_indexes)), dtype=data.dtype)
# iterate through channels to prevent loading all channels into memory which can cause
# memory exhaustion. See https://github.com/SpikeInterface/spikeinterface/issues/3303
for index, channel_index in enumerate(channel_indexes):
    sig_chunk[:, index] = data[channel_index :: self._num_channels]

The stated rationale — avoiding loading all channels into memory — does not
apply, because data has already been fully read into memory by
self._read_function(...) on the line immediately above. The loop therefore
buys no memory saving and costs num_channels strided gathers over an array
that is already resident. On a 4096-channel BioCam that is 4096 cache-hostile
passes per chunk.

Since the array is contiguous and frame-major, a single reshape gives the same
result as a view.

Measured

3Brain BioCam, 4096 channels @ 19,753.775 Hz, BRW4 (Well_A1/Raw, flat uint16),
reading 100,000 frames:

Windows (native h5py) WSL2 over DrvFs
stock neo 93 MB/s 77 MB/s
with the patch below 214 MB/s (2.8×)
raw h5py + reshape ceiling 1,647 MB/s 328 MB/s

Output verified bit-identical (np.array_equal) between stock and patched
paths.

Suggested fix

else:
    nch = self._num_channels
    if data.size % nch:
        # unexpected layout - fall back rather than mis-shape the data
        ...existing per-channel loop...
    else:
        arr = data.reshape(-1, nch)
        if channel_indexes is None or (
            isinstance(channel_indexes, slice) and channel_indexes == slice(None)
        ):
            sig_chunk = arr
        else:
            sig_chunk = arr[:, channel_indexes]

The event-based-compressed branch (readHDF5t_brw4_sparse) is untouched.

If the memory concern in issue #3303 is really about _read_function loading a
whole chunk, that is worth addressing separately — but it should be fixed there,
by reading a channel subset from HDF5, rather than by re-gathering data that is
already in RAM.

Working patch

A monkey-patch implementing exactly this, with a verify() that asserts
bit-identical output and reports the speed-up, is available and can be turned
into a PR on request.

Environment

neo as shipped with SpikeInterface 0.104.8, h5py 3.16.0, Python 3.12,
Ubuntu 24.04 (WSL2) and Windows 11.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions