search

flatironinstitute / neurosift

Browser-based NWB visualization and DANDI exploration
Apache License 2.0
45 stars 6 forks source link

TimeSeries: handle case where rate=0 #58

Closed magland closed 1 year ago

magland commented 1 year ago

In the following example, the TwoPhotonSeries has rate=0

@CodyCBakerPhD @bendichter is this expected?

https://flatironinstitute.github.io/neurosift/#/nwb?url=https://dandiarchive.s3.amazonaws.com/blobs/24b/910/24b9109f-ba63-462f-a2a6-c051e63e4537

I guess I will treat the 0 as a 1 to avoid division by zero.

bendichter commented 1 year ago

No this is not expected. Cody, do we have a check for this in the inspector? Any idea why someone would set this to 0?

CodyCBakerPhD commented 1 year ago

Cody, do we have a check for this in the inspector?

Nope, I would have assumed PyNWB would have prevented that (ofc could be created via MatNWB...), and I don't think I've ever seen it myself. Oh man, I just tried it (mock_TimeSeries(rate=0.0)) and it seems fine. Raising issues on Inspector and PyNWB for this

It is also a common mistake for people to invert their sampling rate (interpreting it as a period in units seconds) or not store it in Hz as defined in the schema (https://nwb-schema.readthedocs.io/en/latest/format.html#timeseries), but that's a tricky thing to check for programmatically because I've seen inverted values that are also within the range of non-inverted ones, so no Inspector check for that either - but it's easy to spot in visualizations relative to other data streams in the file

CodyCBakerPhD commented 1 year ago

Though as I look deeper into this file I find several other issues - these 'TwoPhotonSeries' do not look like the same imaging plane emitting variable fluorescence over time, they look like a static image of a volume (with the first axis being depth?) which would also explain the rate=0.0

There are technically some other core NWB data types for static images but they don't interface with microscopy metadata so this is really a case where the user should have used an extension (Ryan was working with someone a while back on this, not sure where it ended up). But in that case it may have seemed like too much work so the TwoPhotonSeries could technically be harnessed for this purpose (albeit with the correct axes order - all their demo code is in MATLAB so I'm guessing that's what they used to write - which has well known transposition issues)