Skip to content

Minimal fix of SpikeGadgetsRawIO channel ID issue #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 24, 2024
Merged

Minimal fix of SpikeGadgetsRawIO channel ID issue #1496

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a6d64d
Minimal fix of SGRawIO channel id
khl02007 Jun 26, 2024
38a012b
clean up PR
zm711 Jul 10, 2024
69b5403
Merge branch 'master' into sgrawio_chan_fix
zm711 Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions neo/rawio/spikegadgetsrawio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@
Author: Samuel Garcia
"""

import numpy as np

from xml.etree import ElementTree

from .baserawio import (
BaseRawIO,
_signal_channel_dtype,
_signal_stream_dtype,
_spike_channel_dtype,
_event_channel_dtype,
)

import numpy as np

from xml.etree import ElementTree
from neo.core import NeoReadWriteError


class SpikeGadgetsRawIO(BaseRawIO):
Expand Down Expand Up @@ -79,6 +80,24 @@ def __init__(self, filename="", selected_streams=None):
def _source_name(self):
return self.filename

def _produce_ephys_channel_ids(self, n_total_channels, n_channels_per_chip):
"""Compute the channel ID labels
The ephys channels in the .rec file are stored in the following order:
hwChan ID of channel 0 of first chip, hwChan ID of channel 0 of second chip, ..., hwChan ID of channel 0 of Nth chip,
hwChan ID of channel 1 of first chip, hwChan ID of channel 1 of second chip, ..., hwChan ID of channel 1 of Nth chip,
...
So if there are 32 channels per chip and 128 channels (4 chips), then the channel IDs are:
0, 32, 64, 96, 1, 33, 65, 97, ..., 128
See also: https://github.com/NeuralEnsemble/python-neo/issues/1215
"""
ephys_channel_ids_list = []
for hw_channel in range(n_channels_per_chip):
hw_channel_list = [
hw_channel + chip * n_channels_per_chip for chip in range(int(n_total_channels / n_channels_per_chip))
Copy link
Contributor

Choose a reason for hiding this comment

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

will this int(n_total_channels / n_channels_per_chip) works for any n_total_channels numbers.
Do we need an explicit ceil or floor ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be good. Most of the times you'll have increments of 32 channels/chip. So total channels would have to be some multiple of 32. I don't know gadgets well enough, but I guess the problem would be Intan does sell 16 channel headstage so if you were allowed to mix headstage chips then this math wouldn't work. But I think in this case we wait for a user to open an issue so we have mixed headstage data to work with.

]
ephys_channel_ids_list.append(hw_channel_list)
return [channel for channel_list in ephys_channel_ids_list for channel in channel_list]

def _parse_header(self):
# parse file until "</Configuration>"
header_size = None
Expand All @@ -104,6 +123,20 @@ def _parse_header(self):
self._sampling_rate = float(hconf.attrib["samplingRate"])
num_ephy_channels = int(hconf.attrib["numChannels"])

# check for agreement with number of channels in xml
sconf_channels = np.sum([len(x) for x in sconf])
if sconf_channels < num_ephy_channels:
num_ephy_channels = sconf_channels
if sconf_channels > num_ephy_channels:
Copy link

@jnjnnjzch jnjnnjzch Nov 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry to interrupt, but is that necessarry to add a comparing that num_chan_per_chip > num_ephy_channels?
I got a recording recently and found that num_chan_per_chip is ocasionally read as 339134394.
I manage to extract the xml file and see
<SpikeConfiguration categories="" chanPerChip="339134394" device="intan">
which implies some error...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean the workspace file you use to collect data has chanPerChip="339134394"?

Copy link

@jnjnnjzch jnjnnjzch Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, we are using the workspace file with chanPerChip="32", but when we check the output .rec file, it becomes chanPerChip="339134394". We're still trying to figure out what's going on...

Update:
We find out this is a software bug for Trodes. The workspace file should be saved and re-opened before recording. A direct creation and use of workspace file causes this error.

raise NeoReadWriteError(
"SpikeGadgets: the number of channels in the spike configuration is larger than the number of channels in the hardware configuration"
)

try:
num_chan_per_chip = int(sconf.attrib["chanPerChip"])
except KeyError:
num_chan_per_chip = 32 # default value for Intan chips
Copy link
Contributor

Choose a reason for hiding this comment

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

Intan also has 64 and 16 channel headstages. Where is this 32 coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

most spikegadgets products use 64 channel chips, but a 64 channel chip is just two 32 channel chips sharing an output pin so this is a reasonable default for most cases.

Copy link

@jnjnnjzch jnjnnjzch Nov 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

most spikegadgets products use 64 channel chips, but a 64 channel chip is just two 32 channel chips sharing an output pin so this is a reasonable default for most cases.

Again, sorry to interrupt you, @khl02007 ... Is this still work on signal channel level? I mean in practice, does it mean there is no need to change num_chan_per_chip manually into 64 even we know we are using a 64 channel chip, but the header_text xml extracts as sconf.attrib["chanPerChip"] = 32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jnjnnjzch if you are using this product then yes, no need to change num_chan_per_chip. If you're using some other product, I'm actually not sure what would happen here...

Copy link

Choose a reason for hiding this comment

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

Thanks! we are exactly using this product! And your opinion is validated again by checking the signal synchronization of the extracted trace. Thanks a lot!


# explore sub stream and count packet size
# first bytes is 0x55
packet_size = 1
Expand Down Expand Up @@ -174,6 +207,7 @@ def _parse_header(self):
signal_streams.append((stream_name, stream_id))
self._mask_channels_bytes[stream_id] = []

channel_ids = self._produce_ephys_channel_ids(num_ephy_channels, num_chan_per_chip)
chan_ind = 0
self.is_scaleable = "spikeScalingToUv" in sconf[0].attrib
if not self.is_scaleable:
Expand All @@ -190,8 +224,8 @@ def _parse_header(self):
units = ""

for schan in trode:
name = "trode" + trode.attrib["id"] + "chan" + schan.attrib["hwChan"]
chan_id = schan.attrib["hwChan"]
chan_id = str(channel_ids[chan_ind])
name = "hwChan" + chan_id

offset = 0.0
signal_channels.append(
Expand Down
Loading