Skip to content
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.

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

Added Plexon's SiNAPS probe definitions. #10

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

nikhilchandra
Copy link

I have added Plexon's probe definitions for five SiNAPS probes:

1S256 -- this is a 1-shank probe with 256 channels arranged in 2 columns.
1S512 -- this is a 1-shank probe with 512 channels arranged in 2 columns.
1S512-N -- this is a 1-shank probe with 512 channels arranged in 4 columns, intended for non-human primates.
4S1024 - this is a 4-shank probe with 1,024 channels (256 channels per shank). For each shank, channels are arranged in 2 columns.
8S1024 - this is an 8-shank probe with 1,024 channels (128 channels per shank). For each shank, channels are arranged in 2 columns.

@nikhilchandra
Copy link
Author

Hi @alejoe91 I hope you are well. I hope I did everything correctly for adding definitions for SiNAPS probes to Probe Interface Library?

@nikhilchandra
Copy link
Author

Looks like there was an extra property "device_channel_indices" that wasn't supposed to be there. I have removed it.

@alejoe91
Copy link
Member

Indeed. Actually in the case of Sinaps I think it's ok that device channel.indices is specified in the json, since the probes are pre-wired and wiring cannot be changed. @nikhilchandra I'll make a separate PR to fix the validation because I think it makes more sense here.

@samuelgarcia @ZM11 do you guys agree?

Then you can add back the device_channel_indices (but let's wait for Sam's and Zach's feedback)

@nikhilchandra
Copy link
Author

I'm not sure if the device channel indices are even required. So far as I know there is no "rewiring" of channels that takes place between the probe and the base station.

@nikhilchandra
Copy link
Author

@alejoe91 is this ready for merging?

@alejoe91 alejoe91 merged commit bfb5b47 into SpikeInterface:main Nov 18, 2024
1 check passed
@samuelgarcia
Copy link
Member

oups you laready merged this.
I wanted to discuss the wiring hard coded in the file.
Is it possible to have 2 probes ? If yes then the wiring come from the soft ware and not the probe itself.
I would prefer to have the layout unwiring here and make the wiring in read_plewon for instance.
No ?

@alejoe91
Copy link
Member

Wiring was removed. The probes are not switchable, bust still I think that wiring should be added when reading the file/processing the data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants