catalystneuro / neuroconv

Create NWB files by converting and combining neural data in proprietary formats and adding essential metadata.
https://neuroconv.readthedocs.io
BSD 3-Clause "New" or "Revised" License
46 stars 22 forks source link

[Feature]: Option for multiple DLC inputs #915

Open vigji opened 1 month ago

vigji commented 1 month ago

What would you like to see added to NeuroConv?

I am trying to load multiple videos and DLC files from an experiment where different cameras with different frame rates etc look at different parts of the animal. as a result, I have something like:

animal1data
   |_video1.avi
   |_video1DLCetc.h5
   |_video2.avi
   |_video2DLCetc.h5

DLC_models_dir
  |_model_for_video1/config.yaml
  |_model_for_video2/config.yaml

I can load multiple camera videos passing a list to the VideoInterface. Already here it feels like I'm stretching the class' domain, as from what I understand this would be oriented toward multiple segments in time, correct?

On the other hand, it is not clear to me if it is possible to load multiple DLC tracking results. I tried to have multiple DeeplabcutInterface objects passed to the ConverterPipe combinator but it complains about having multiple data of the same category (ValueError: 'PoseEstimation' already exists in ProcessingModule 'behavior')

Is your feature request related to a problem?

I cannot find a way to implement multiple DLC labelling in the same file.

Do you have any interest in helping implement the feature?

Yes.

Code of Conduct

CodyCBakerPhD commented 1 month ago

We rely on DLC2NWB for this functionality; I see you've already started contributing some code to that

Would you be willing to submit PRs on their end to allow control over the name of the PostEstimation object at this line (https://github.com/DeepLabCut/DLC2NWB/blob/main/dlc2nwb/utils.py#L223)?

As shown in the README for ndx-pose: https://github.com/rly/ndx-pose?tab=readme-ov-file#ndx-pose-extension-for-nwb

vigji commented 1 month ago

Open to try anything but

allow control over the name of the PostEstimation object at this line

I think you will have to elaborate a bit more as I did not get this point and how I should procede - I did not find pointers in the ndx-pose README (probably you pasted twice the same link?)

CodyCBakerPhD commented 1 month ago

I did not find pointers in the ndx-pose README (probably you pasted twice the same link?)

Whoops, updated

Basically just inject a keyword argument for post_estimation_name that gets set at that line as

PoseEstimation(
    name=post_estimation_name,
    ...
)
vigji commented 1 month ago

Thanks! I'll ask over there if I can contribute this feature as well, I would leave this open until I can close things there!

vigji commented 1 month ago

So, then I would have to modify neuroconv.datainterfaces.behavior.deeplabcut.py to get a name kwarg (or general optional kwargs) when calling DeepLabCutInterface.add_to_nwbfile(), correct?

CodyCBakerPhD commented 1 month ago

Yep, exactly

vigji commented 1 month ago

Sorry if I am slow on this, but I need another pointer. To my understanding, if I use the PipeInterface to run the combined conversion of 2 DeepLabCutInterface and 1 VideoInterface object upstream of the call to DeepLabCutInterface.add_to_nwbfile(), I would need to use the conversion_options argument of ConverterPipe.run_conversion() to pipe down the name. But what keys should I use to populate the dictionary? Until now, the two dlc_interface objects are identical no?

My code:

conv_pipe = ConverterPipe([dlc_interface1, dlc_interface2, video_interface], verbose=True)
conversion_options = dict(...) # ...?
# Choose a path for saving the nwb file and run the conversion
conv_pipe.run_conversion(nwbfile_path=path_to_save_nwbfile, metadata=metadata,
                         conversion_options=conversion_options)

[Edit add] Maybe one option could be to resave the file at multiple steps and add content one interface at the time but that is not a supported flow at the moment right?

CodyCBakerPhD commented 1 month ago

[Edit add] Maybe one option could be to resave the file at multiple steps and add content one interface at the time but that is not a supported flow at the moment right?

You can always do that

I would need to use the conversion_options argument of ConverterPipe.run_conversion() to pipe down the name.

Yes, this would be a conversion option

But what keys should I use to populate the dictionary?

If you used a NWBConverter instead, you would use whatever key you defined in the data_interface_classes

Until now, the two dlc_interface objects are identical no?

This bit a code: https://github.com/catalystneuro/neuroconv/blob/main/src/neuroconv/nwbconverter.py#L290-L295

disambiguates the name based on their order in the data_interfaces list of the ConverterPipe, so you'd just need to check which is which in the automatically constructed data_interface_classes attribute

vigji commented 1 month ago

Awesome, thanks a lot!

Then I'll submit a PR with the name argument piping.

Already here it feels like I'm stretching the class' domain, as from what I understand this would be oriented toward multiple segments in time, correct?

For the videos: is my above reading correct? If yes, should I use a single interface passing a list of videos, or find a similar solution with one VideoInterface per video?

CodyCBakerPhD commented 1 month ago

Multiple file paths per interface is for when the stream from a single camera is broken up into different files over time, either contiguous or distinct segments

Separate cameras need separate interfaces though, so your usage is spot on in that respect

vigji commented 1 month ago

But this would require some modification of the existing VideoInterface right?

If I try to implement it with current class, at the moment of calling add_to_nwbfile for the first interface I get a mismatch between the number of file linked (1) and the number of video metadata around in ["Behavior"]["Videos"], since that dictionary is updated by the instantiation of both objects, correct? (at least I find it populated with both without me explicitly adding anything, so I assume it is happening under the hood)

CodyCBakerPhD commented 1 month ago

I'm talking about the DLC interfaces

The VideoInterfaces also have a similar problem as you point out, which has already been raised in #905 and PR open in #914