Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Add optional device linking to TimeSeries #602

Open
3 tasks done
h-mayorquin opened this issue Dec 11, 2024 · 2 comments
Open
3 tasks done

[Feature]: Add optional device linking to TimeSeries #602

h-mayorquin opened this issue Dec 11, 2024 · 2 comments
Assignees
Labels
category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of NWB users
Milestone

Comments

@h-mayorquin
Copy link
Contributor

h-mayorquin commented Dec 11, 2024

This relates to writing data from NIDQ devices in the context of a SpikeGLX recording. A more complete discussion can be found here.

To summarize:

Analog channels from NIDQ devices can represent a variety of non-neural signals (e.g., motion, temperature, MEG). It’s unclear how best to store this information. One option is to save every channel as a separate TimeSeries (e.g., TimeSeriesMotionChannel1, TimeSeriesTemperatureChannel2, TimeSeriesMEGChannel3, etc.). This approach allows the TimeSeries description to include all necessary information and also to use other fields such as gains, offsets, and units for better provenance.

However, this approach has a downside: there is currently no formal way to indicate that all of these TimeSeries were recorded from the same device (the NIDQ board). I think it would be good for provenance if we had the option to link a TimeSeries object to a Device.

What do you think? Am I missing something?

Code of Conduct

@rly rly transferred this issue from NeurodataWithoutBorders/pynwb Dec 12, 2024
@rly rly added this to the Future milestone Dec 12, 2024
@rly rly added category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of NWB users labels Dec 12, 2024
@stephprince
Copy link
Contributor

thanks for the issue @h-mayorquin

@rly @oruebel and I discussed this yesterday and think it's a good idea but we would need to investigate further to make a plan for the exact implementation.

One point that came up was that there are some TimeSeries that are not directly generated from a device but instead from a preprocessing step or script. In these cases having the option to link a device may not be appropriate.

One option to address this is to create a new intermediate TimeSeries type (e.g. "RecordedTimeSeries") that allows this optional device linking. To start implementing this approach, we should first take a look through all the types that inherit from TimeSeries and determine which ones would be a "RecordedTimeSeries" with optional Device linking vs. the current definition of TimeSeries without Device linking.

@h-mayorquin
Copy link
Contributor Author

h-mayorquin commented Dec 13, 2024

Hi guys, thanks for discussing my issue.

Yes, I thought that a sub-type of TimeSeries would be less disruptive since TimeSeries is so fundamental. You might not want a device on some of the children of TimeSeries for example. Slightly related would be the proposal of @rly here to separate Neural from Non-neural ElectricalSeries which could solve this problem as well (although, note the NIDQ electrical series should not be linked to electrodes).

The other possible solution to the NIDQ metadata problem would be to add a way of annotating channels. That way, I can use a single TimeSeries but then annotate the modality of each channel there. This goes close to the Xarray model of labeling dimensions and rows which might be nice but I guess that would require a big discussion among you guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: enhancement improvements of code or code behavior priority: medium non-critical problem and/or affecting only a small set of NWB users
Projects
None yet
Development

No branches or pull requests

3 participants