search

NovelaNeuro / ndx-franklab-novela

BSD 3-Clause "New" or "Revised" License
1 stars 1 forks source link

NwbImageSeries contains Devices but should link to them #68

Open rly opened 3 years ago

rly commented 3 years ago

The NwbImageSeries type contains a collection of Device types. This will nest the Device under NwbImageSeries. This is OK, but to fit NWB best practices better, all Device types should live under the path /general/devices which is done in PyNWB via nwbfile.create_device(name='dev0') and the NwbImageSeries type should link to those Devices.

Current:

- neurodata_type_def: NwbImageSeries
  neurodata_type_inc: ImageSeries
  doc: Extension of ImageSeries object in NWB
  groups:
  - neurodata_type_inc: Device
    doc: devices used to record video
    quantity: '*'

Suggestion:

- neurodata_type_def: NwbImageSeries
  neurodata_type_inc: ImageSeries
  doc: Extension of ImageSeries object in NWB
  links:
  - target_type: Device
    doc: devices used to record video
    quantity: '*'

@lfrank This extended NwbImageSeries is currently set up to associate an ImageSeries with multiple devices. When would you want to link more than one device with the same NwbImageSeries? Would quantity: ?, which means zero or one devices are present, be more appropriate?

Note that NWB 2.3.0 adds to the base ImageSeries type an optional link to one Device. If this is sufficient, then we can update the extension to remove this type. If it makes sense to associate it with multiple devices, then we can consider amending the NWB schema to allow that.

lfrank commented 3 years ago

Hi Ryan. That change makes sense to me, and right now we have a single timeseries per device, but note that with multiple cameras operating simultaneously it could be that one would create a joint time series that had data from multiple devices.

Would this change be easy for you to make?

Loren

On Jun 3, 2021, at 1:53 PM, Ryan Ly @.***> wrote:

The NwbImageSeries type contains a collection of Device types. This will nest the Device under NwbImageSeries. This is OK, but to fit NWB best practices better, all Device types should live under the path /general/devices which is done in PyNWB via nwbfile.create_device(name='dev0') and the NwbImageSeries type should link to those Devices.

Current:

  • neurodata_type_def: NwbImageSeries neurodata_type_inc: ImageSeries doc: Extension of ImageSeries object in NWB groups:

    • neurodata_type_inc: Device doc: devices used to record video quantity: '*' Suggestion:

  • neurodata_type_def: NwbImageSeries neurodata_type_inc: ImageSeries doc: Extension of ImageSeries object in NWB links:

    • target_type: Device doc: devices used to record video quantity: '*' @lfrank https://github.com/lfrank This extended type is currently set up to associate an ImageSeries with multiple devices. When would you want to link more than one device with the same TimeSeries? Would quantity: ?, which means zero or one devices are present, be more appropriate?

Note that NWB 2.3.0 adds to the ImageSeries type an optional link to one Device. If this is sufficient, then we can update the extension to remove this type. If it makes sense to associate it with multiple devices, then we can consider amending the NWB schema to allow that.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NovelaNeuro/ndx-franklab-novela/issues/68, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABV4PSOYI67BB2PO5NDHFZDTQ7TUTANCNFSM46BP5NOQ.

rly commented 3 years ago

What would be the advantage of creating a single ImageSeries with data combined from multiple cameras? With multiple cameras operating simultaneously, I think it would be better to store the data as separate ImageSeries. The cameras may have different frame rates / timestamps, which is not allowed under a single ImageSeries, and if the data from multiple cameras are merged into a single ImageSeries, it would not be possible to distinguish which image came from which camera. (The dimensions of ImageSeries do not allow for specifying another axis for device).

If you really want to combine them into a single data type that uses the same timestamps, then I think we should change this extension NwbImageSeries data type to specify that the second dimension is the camera.

lfrank commented 3 years ago

Oh, sorry, I wasn’t clear. The iidea would be some sort of, for example, 3D model of the animal based on multiple cameras. But given that I would think that sort of thing should be created using something like DataJoint, I think for NWB it would be fine to have a single device per timeseries.

Loren

On Jun 3, 2021, at 3:53 PM, Ryan Ly @.***> wrote:

What would be the advantage of creating a single ImageSeries with data combined from multiple cameras? With multiple cameras operating simultaneously, I think it would be better to store the data as separate ImageSeries. The cameras may have different frame rates / timestamps, which is not allowed under a single ImageSeries, and if the data from multiple cameras are merged into a single ImageSeries, it would not be possible to distinguish which image came from which camera. (The dimensions of ImageSeries do not allow for specifying another axis for device).

If you really want to combine them into a single data type that uses the same timestamps, then I think we should change this extension NwbImageSeries data type to specify that the second dimension is the camera.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NovelaNeuro/ndx-franklab-novela/issues/68#issuecomment-854237146, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABV4PSPSDFMCKAC742AG3XDTRABWBANCNFSM46BP5NOQ.

rly commented 3 years ago

OK, thanks for the clarification!