LorenFrankLab / trodes_to_nwb

Converts data from SpikeGadgets to the NWB Data Format
MIT License
2 stars 2 forks source link

stop using ntrode #42

Closed khl02007 closed 1 year ago

khl02007 commented 1 year ago

@rly @samuelbray32 @edeno I noticed that the yaml for spikegadgets_to_nwb, like rec_to_nwb, asks users to define ntrode_electrode_group. can we not do this? an ntrode is something that experimenters define for visualization purposes on Trodes, and as such different people have different conventions for setting ntrodes. the conversion shouldn't take that into account. I suggest that we just work with hwchan.

samuelbray32 commented 1 year ago

This issue of ntrodes groupings is addressed using the reconfig file. Since the same reconfig is used for every session of an animal isn't this less work and less susceptible to user error than entering the 100+ hwChan for each session's metadata yaml?

khl02007 commented 1 year ago

I thought in the new version we are not doing reconfig - instead we are accessing data from the rec file directly via spikeinterface. Is that not the case? I agree that for high channel count data, people should programmatically specify the electrodes and their locations etc. I like defining probes using probeinterface (and save them as json files), creating a probegroup from them and attaching that to the recording. There is a NDX for it that I helped a bit with that should work for writing it to NWB (https://github.com/SpikeInterface/ndx-probeinterface)

edeno commented 1 year ago

Hi @khl02007 , could you explain more about how you are using probe interface? We are indeed reading directly from the rec file (as spikegadgets/neo does, but with some additional functions to read), but we are also utilizing the reconfig. This way you are doing things seems to be a different way from anyone else in the lab.

I think we would be open to exploring things like this but of course we need to build things for the way most of the lab does things, so it would be good to know if this way is simpler for people or not.

khl02007 commented 1 year ago

@edeno Sorry I wasn't clear. I thought that the reconfig refers to the -reconfig flag of trodesexport, which (I thought) we are no longer using. The main issue here seems to be how to associate the recording (read via spikeinterface) with a set of channel-related information, like the locations. My impression (maybe I'm wrong) is that we are currently using trodes config file for that and passing that as a 'reconfig' file. My suggestion is that using probeinterface is better for keeping track of that kind of information because it is better defined and easier to manipulate. For example, here is how I read my recording and then associate with it a set of probes. The recording is made with two Livermore probes.

import spikeinterface as si
import probeinterface as pi

# generate probe object (just need to do once)
def get_Livermore_15um(device_channel_indices: Union[str,List]='SpikeGadgets', order:int=0, shift=[0,0]):
    """Returns a probeinterface Probe object of the Livermore 15um probe plugged into SpikeGadgets H70 probe board

    Parameters
    ----------
    device_channel_indices : str or iterable
        The mapping from the contact IDs to device channel indices; depends on the hardware, by default assumes SpikeGadgets H70 probe board
    order : int
        order in a probegroup, by default 0
    shift : list, optional
        how much to shift the channel locations by, by default [0,0] (important for probegroup)
    """
    Livermore_15um = pi.generate_multi_shank(num_shank=4, shank_pitch=[250, 0],
                                             num_columns=1, num_contact_per_column=32,
                                             xpitch=0, ypitch=26, y_shift_per_column=None,
                                             contact_shapes='circle', contact_shape_params={'radius': 15/2})

    # flip because electrodes should be facing you
    contact_ids = np.concatenate([np.arange(96,128), np.arange(64,96), np.arange(32,64), np.arange(0,32)])
    contact_ids = contact_ids + order*128
    Livermore_15um.set_contact_ids(contact_ids)

    device_channel_indices = np.array([35, 39, 43, 47, 50, 54, 58, 62, 4, 0, 5, 32, 12, 20, 17, 8, 34, 38, 42, 46, 51, 55, 59, 63, 24, 16, 13, 9, 21, 28, 25, 29,
                                       33, 37, 41, 45, 48, 52, 56, 61, 18, 22, 26, 30, 3, 1, 7, 15, 31, 36, 40, 44, 49, 53, 57, 60, 11, 2, 6, 10, 14, 19, 23, 27,
                                       124, 126, 120, 112, 109, 105, 101, 97, 79, 75, 71, 66, 94, 90, 86, 82, 116, 125, 121, 117, 113, 108, 104, 100, 95, 91, 87,
                                       83, 78, 74, 70, 67, 115, 107, 110, 119, 123, 127, 122, 96, 77, 73, 69, 65, 92, 88, 84, 80, 103, 111, 114, 118, 106, 99, 102,
                                       98, 93, 89, 85, 81, 76, 72, 68, 64])
    device_channel_indices = device_channel_indices + order*128
    Livermore_15um.set_device_channel_indices(device_channel_indices)
    Livermore_15um.move(shift)
    return Livermore_15um

recording = si.read_spikegadgets(...)

probe_right_CA1 = get_Livermore_15um(order=0)
probe_left_V1 = get_Livermore_15um(order=1, shift=[-np.sqrt(4000**2+5000**2), 1300])

probegroup = pi.ProbeGroup()
probegroup.add_probe(probe_right_CA1)
probegroup.add_probe(probe_left_V1)

recording = recording.set_probegroup(probegroup)

How would you do something like this with the trodes config - reconfig approach? Do we have a standard for how to specify probe related information in the config file?

I was also just confused by ntrode because it can be any random grouping of electrodes. Specifically it doesn't convey information about whether it is a shank, a probe, etc. For example in my recordings, an ntrode is a group of four electrodes that are nearby. I use four just for historical reasons and because if too many electrodes are included, the computer has difficulty displaying the spike view. Of course these concerns shouldn't matter when converting to NWB, which was my original point.

If you feel like the current approach works, then i would at the minimum suggest that we clearly define what an ntrode is and also have some functions that can help the user easily recreate a config file that has different ntrode groupings for an arbitrary number of probes / tetrodes.

edeno commented 1 year ago

Hi @khl02007,

Thank you for the suggestion. @samuelbray32 and I talked about this and the way you are specifying things in spikeinterface is already handled in the current configuration. We already have a yaml file that carries information about the shank, probe grouping. This is a separate yaml file which is referred to in the main yaml file by the device type (e.g. tetrode_12.5, 128c-4s8mm6cm-20um-40um-sl, etc.).

We are happy to help you understand how this works for your data (@samuelbray32 has a really good handle on this so I'm sure it would take 5 minutes of chatting), but implementing the new feature via spikeinterface would be more work for little gain in functionality and delay the rollout of spikegadgets_to_nwb. In addition, it would confuse the rest of the lab as they are already used to the current configuration.

I am going to close this issue for now, but if you still feel like we are missing something, feel free to reopen.