Minimal fix of SpikeGadgetsRawIO channel ID issue
#1496
Conversation
| try: | ||
| num_chan_per_chip = int(sconf.attrib["chanPerChip"]) | ||
| except KeyError: | ||
| num_chan_per_chip = 32 # default value for Intan chips |
There was a problem hiding this comment.
Intan also has 64 and 16 channel headstages. Where is this 32 coming from?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@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...
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Just one error correction to implement and then a discussion on the ephys channels function. I'll also have Alessio be a second pair of eyes, but once the error is fixed I would be fine with this (with a slight preference to tidying up the function either with my comment or switching to better explained for loops).
|
the only thing preventing this from getting merged from my perspective is using the appropriate error message. Would you like to do that fix? |
|
@alejoe91, I've added my cleanup to this PR. So I'm okay with this. Do you know the spikegadgets format well enough to double check the logic of this? |
| 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)) |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
|
Hi Kyu, |
|
@zm711 @samuelgarcia sorry I forgot about this PR. In general I'm happy to let you guys make any changes to the PR as long as the functionality is preserved (which you seem to have done). Thanks and hopefully we can access spikegadgets rec files via spikeinterface now. |
| 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: |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
do you mean the workspace file you use to collect data has chanPerChip="339134394"?
There was a problem hiding this comment.
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...
Only the part from #1303 that fixes the channel id issue from #1215
Hopefully this can be merged easily