search

fNIRS / snirf

SNIRF Format Specification
http://fnirs.org/resources/software/snirf/
Other
59 stars 34 forks source link

/nirs(i)/stim(j)/data does not use TimeUnit #111

Open rob-luke opened 2 years ago

rob-luke commented 2 years ago

The specification uses the field TimeUnit to specify the unit that times are stored in. This field is used in many places including /nirs(i)/data(j)/time, /nirs(i)/probe/timeDelays, /nirs(i)/aux(j)/time etc.

However, the /nirs(i)/stim(j)/data field specifies that seconds must be used. It states (accentuation my own)...

This is a numeric 2-D array with at least 3 columns, specifying the stimulus time course for the jth condition. Each row corresponds to a specific stimulus trial. The first three columns indicate [starttime duration value]. The starttime, in seconds, is the time relative to the time origin when the stimulus takes on a value; the duration is the time in seconds that the stimulus value continues, and value is the stimulus amplitude. The number of rows is not constrained. (see examples in the appendix).

This seems counterintuitive to me. Ideally the same time unit would be used all throughout the specification. Unfortunately, changing this would be a breaking change. So I am tagging this as "breaking change" and "Version 2" so that we track this inconsistency and can possibly fix it in the future.

rob-luke commented 2 years ago

As noted in https://github.com/mne-tools/mne-python/issues/10538#issuecomment-1106636185

samuelpowell commented 4 months ago

Reviewed in meeting of May 24 and confirmed that this change should be implemented in v2.