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
51 stars 23 forks source link

[Bug]: ndx_pose timestamps #521

Closed jeongjunjjkim closed 7 months ago

jeongjunjjkim commented 1 year ago

What happened?

Converting DeepLabCut h5 files into nwb generates timestamps based on frames rather than frame rate. e.g. 8000 frames, 400Hz tracked video leads to timestamps of 1-8000 s on ndx-pose rather than 1-20s.

Steps to Reproduce

interface_dlc = DeepLabCutInterface(file_path=file_path, config_file_path=config_file_path, subject_name=animalId, verbose=False)

    metadata = interface_dlc.get_metadata()
    # For data provenance we add the time zone information to the conversion
    session_start_time = datetime(2022, 10, 26, 9, 30, 0, tzinfo=tz.gettz("US/Eastern"))
    metadata["NWBFile"].update(session_start_time=session_start_time)
    # Choose a path for saving the nwb file and run the conversion
    nwbfile_path = f"./dlc_nwb_convert/{dlcls[i]}.nwb"  # This should be something like: "./saved_file.nwb"
    interface_dlc.run_conversion(nwbfile_path=nwbfile_path, metadata=metadata)

Traceback

No response

Operating System

Windows

Python Executable

Conda

Python Version

3.9

Package Versions

No response

Code of Conduct

CodyCBakerPhD commented 1 year ago

I believe this is intentional but maybe @h-mayorquin can confirm

I recall a lot of work being done regarding how DLC interacts with OpenCV and how timestamps come into that: https://github.com/DeepLabCut/DLC2NWB/pull/16

h-mayorquin commented 1 year ago

@jeongjunjjkim Can you show your traceback? Is your video file available?

If your video file is available then dlct2nwb will extract the timestamps from there. If not, then it will just use the frames as timestamps:

https://github.com/DeepLabCut/DLC2NWB/blob/0dd3d16e240f45518e8fe040a9f8779c2c8f60fc/dlc2nwb/utils.py#L177-L186

I think this is what you are seeing.

h-mayorquin commented 1 year ago

@CodyCBakerPhD About the following I am less confident but from memory the context of the situation is the following: The problem with this -and happens in sleap as well- is that the PoseEstimationSeries do not have a uniform sampling rate. The reason being, that a specific node (say an arm) might not get a sucesfully prediction of position at any given frame so that is skipped. This means that they are not uniform so it is wrong to write sampling rate.

On the other hand, the video is usually available at analysis time (when people make the prediciton of positions) but not at conversion time. So the timestamps are usually not availalable.

The best solution would be for the formats to keep this information in their provenance files but this is not yet done and I am not sure that's a prioirty.

Meanwhile, when I did sleap I left a way of passing general metadata to the nwbfile:

https://github.com/talmolab/sleap-io/blob/430a061233a5ab98f60d353e528e5a2652de5ca8/sleap_io/io/nwb.py#L201-L225

But when this was done for dlc2nwb we were not so timestamp savy so nobody though about that. Probably this will be the way to go to add full time awareness to DeepLabCutInterface (which should not be inherting from BaseTemporalAlignmentInterface yet!)

jeongjunjjkim commented 1 year ago

@h-mayorquin Ah that makes sense - I just used the h5 file without the video file. On a related note, more about feature, rather than bug, it would be nice to have a method to set timestamps (arbitrarily). Often folks have continuous ephys recordings but discontinuous, trial-based video recordings. I see this is implemented in VideoInterface but not DLC Interface.

CodyCBakerPhD commented 1 year ago

Can you show your traceback? Is your video file available?

I don't think it's causing an error, it sounds like it writes to the file it's just the value of the contents is incorrect (or rather, the units of the timestamps is incorrect, perhaps it should be 'samples')

h-mayorquin commented 1 year ago

@CodyCBakerPhD There should be a warning that indicates that the video file is not available as expected but he already confirmed. that.

@jeongjunjjkim Yes, that's our goal. Most of our interfaces support setting timestamps but is hidden under alignment: https://neuroconv.readthedocs.io/en/main/user_guide/temporal_alignment.html The other comment addressed to @CodyCBakerPhD was meant to delineate how can add suppport for these time setters on this specific interface.

h-mayorquin commented 7 months ago

OK, So we provided and explanation and now DeepLabCutInterface has the the set_aligned_timestamps method:

https://github.com/catalystneuro/neuroconv/blob/0d03d891e68c80c91b0f9bd3509e581e1e898412/src/neuroconv/datainterfaces/behavior/deeplabcut/deeplabcutdatainterface.py#L113-L123

I will close this issue as solved