Skip to content

Use probeinterface's renamed Neuropixels readers and new probe detectors#4593

Open
h-mayorquin wants to merge 3 commits into
SpikeInterface:mainfrom
h-mayorquin:neuropixels_use_new_probe_extraciton_functions
Open

Use probeinterface's renamed Neuropixels readers and new probe detectors#4593
h-mayorquin wants to merge 3 commits into
SpikeInterface:mainfrom
h-mayorquin:neuropixels_use_new_probe_extraciton_functions

Conversation

@h-mayorquin

Copy link
Copy Markdown
Collaborator

probeinterface renamed read_openephys to read_openephys_neuropixels (SpikeInterface/probeinterface#427) and read_spikegadgets to read_spikegadgets_neuropixels (SpikeInterface/probeinterface#440) to make the Neuropixels-only scope of those readers explicit; the old names remain as deprecation aliases. The same two PRs add detectors has_neuropixels_probes(settings_file, stream_name=...) and has_spikegadgets_neuropixels_probes(file) that parse the settings XML / .rec header and return whether the recording carries any Neuropixels probe geometry.

This PR switches the OpenEphys and SpikeGadgets extractors to call the new names and gates probe attachment on the detectors. Previously both readers were called unconditionally with raise_error=False, so a recording that legitimately had no Neuropixels probe (Intan / Rhythm FPGA / NI-DAQmx for Open Ephys, tetrodes / ECU for SpikeGadgets) silently returned None and the extractor finished probe-less. With the detector check up front the no-probe path is a normal control-flow branch, and any real parsing error from the reader now surfaces instead of being swallowed.
I
I am also removing the version guard and the packaging import, since pyproject.toml already pins probeinterface>=0.3.2.

@chrishalcrow

Copy link
Copy Markdown
Member

Hello, this PR demonstrates an awkward difference between uv and traditional python venvs.

uv checks all dependency groups and prioritizes git repos over pypi releases. E.g. if you run uv run ... uv will create a venv with probeinterface from source, since this is in common-tests
If you make a venv and pip install spikeinterface from source from this PR, the resolver completely ignore common-tests and installs probeinterface v0.3.2.

The new functions are only in probeinterface main. So if you try to load a openephys neuropixel recording with a uv generated venv, it's fine. If you do it with a pip installed venv, it errors.

Just wanted to point it out!

@alejoe91 alejoe91 added the extractors Related to extractors module label May 27, 2026
@h-mayorquin

Copy link
Copy Markdown
Collaborator Author

Hello, this PR demonstrates an awkward difference between uv and traditional python venvs.
...

Just wanted to point it out!

Maybe we should pin to github on probeinterface and python-neo on main. Probably worth its own issue.

@alejoe91

Copy link
Copy Markdown
Member

Hello, this PR demonstrates an awkward difference between uv and traditional python venvs.
...
Just wanted to point it out!

Maybe we should pin to github on probeinterface and python-neo on main. Probably worth its own issue.

Good idea! Let's make a separate PR for it.

@h-mayorquin I'll try to add adc sample shifts/saturation for spikegadgets in the same PR, so everything is unified.

Comment on lines +77 to +78

if len(sample_shifts) > self.get_num_channels():

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

should this be equal?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pups yes of course! Good catch!

sample_shifts = np.array([])
saturation_thresholds_uV = []
for probe in probegroup.probes:
sample_shifts_probe = get_neuropixels_sample_shifts_from_probe(probe)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The function get_neuropixels_sample_shifts_from_probe returns the sample shifts in probe (contact) order, so before you set them as a channel property you need to re-wire them into channel order. This works for SpikeGLX and OpenEphys without any extra step because probeinterface already wires those probes so their contacts are in the order of the binary traces (device_channel_indices = arange), so contact order already equals channel order.

We could do that re-wiring here too, but it is more complicated than in the SpikeGLX and OpenEphys case. Those write one file per probe, while SpikeGadgets puts more than one probe in a single file. For the dataset with neuropixels that we have on gin already (SpikeGadgets_test_data_2xNpix1.0_20240318_173658.rec, the two probes are interleaved in that single binary in blocks of 32 channels (channels 0-31 are probe 0, 32-63 are probe 1, 64-95 are probe 0, and so on), so a probe's contacts do not map to a contiguous run of channels and device_channel_indices is a real permutation (hwChan) rather than arange. That is why the current np.concatenate of per-probe shifts lands on the wrong channels (704 of 768 on this fixture).

I see three possible paths forward:

  1. Re-route the output of get_neuropixels_sample_shifts_from_probe through device_channel_indices. Each probe's device_channel_indices tells you which recording channel each contact maps to, so you scatter the per-probe shifts into a channel-length array:
sample_shifts = np.zeros(self.get_num_channels())
for probe in probegroup.probes:
    shifts = get_neuropixels_sample_shifts_from_probe(probe)
    if shifts is not None:
        sample_shifts[probe.device_channel_indices] = shifts   # scatter into channel order
self.set_property("inter_sample_shift", sample_shifts)
  1. Re-sort at the probeinterface level using the global device channel indices, sorting each probe/group so the contacts come out in channel order. I am not sure this is possible, since each probe owns a non-contiguous, interleaved set of channels, so a per-probe arange might not be doable but given that you guys like that pattern maybe you might be able to find it. I don't like the pattern and you know how motivated thinking is a horrible beast.

  2. Use the property from the probe after you set it. This does the rearranging after we set it on the recording (so it is already re-ordered by set_probegroup): read self.get_property("contact_vector")["adc_sample_order"], which is already one value per channel in channel order, and set the property from that.

I honestly prefer 1: explicit is better than implicit, and it will not clash with any of the current Probe refactorings like option 3 might. Option 2 will require more work on your side and I would like to merge this and then do that if you want to move that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extractors Related to extractors module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants