Fix ephys channel ids in spikegadgetsrawio
Fix #1215. Using code from @khl02007 - thanks!
This needs some more testing, but I am proposing this PR to continue the conversation.
I think setting the channel names to HWChan N (where N is channel index) would be clearer.
@samuelgarcia @JuliaSprenger could you please review this? would like this functionality on spikeinterface asap
Hi @khl02007
Can you remove [WIP] from the PR name and see why tests are failing for spikegadgets?
@rly we also ran black on the whole code base and did some cleanup with docstrings and asserts which have led to merge conflicts. If you have the bandwidth to fix those really quick that would be awesome!
raw_unit16 = raw_unit8_mask.reshape(-1).view("int16").reshape(shape) E ValueError: cannot reshape array of size 512 into shape (1024,0)
This is the error causing tests to fail.
@zm711 it seems the test is failing to read analog signals because what is passed to _get_analogsignal_chunk is block_index = 0, seg_index = 0, i_start = 0, i_stop = 1024, stream_index = 0 but stream_index=0 corresponds to Controller_DIO_digital. To get analog signal (ECU_analog) you have to pass stream_index=2. I would suggest changing the input in the test.
stream_index to stream_id mapping:
0 -> Controller_DIO_digital
1 -> ECU_digital
2 -> ECU_analog
3 -> trodes
@khl02007 , thanks for checking. The way the testing works is it auto-detects what the possible streams are based on the reader and then iterates through them. So that means that the way the reader is implemented in this PR is not quite following the appropriate strategy for a Neo reader (ie this would need to be fixed at the Neo reader level). If this is the case it seems like you would need to add some more logic in the reader to prevent the reader itself from making mistakes between analog and digital signaling. Does that make sense?
@zm711 I think I understand, but what exactly needs to be done to address this concern then? How does the reader automatically detect the possible streams? What is the appropriate strategy for a neo reader?
I just talked to @samuelgarcia and he said he worked don this io before. He might be best to help a bit. He said he was having some struggles trying to get the additional streams due to the way the data is set up so it might be a bit more work to get this to work within the Neo structure. I would either reach out to him directly or ping him every once in a while.
@rly and @khl02007.
We discussed this a bit and from what Sam remembers this format would be very hard to fit within the model of analogsignal chunk that Neo uses because of the way that digital channels are stored. The hack that we used for another IO was to extract those values, but not to expose the stream at the Neo machinery level. Then for users that need that info they could just grab the private stream information from a private attribute.
So more concretely previously for the intanrawio to get access for the digital information you would have to do:
reader = IntanRawIO()
reader.parse_header()
digital_data = reader._raw_data['DIGITAL-IN']
So you could do something similar where you store these channels in the raw data container, but you don't add them to channel info for now.
The future goal is to have a separate digitalsignal stream that would allow us more flexibility in loading these types of signals, but that is on the backburner for now.