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

[Inconsistency] Definition of time when <number of time points> = 2 #164

Open
Edouard2laire opened this issue Dec 9, 2024 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@Edouard2laire
Copy link
Contributor

Edouard2laire commented Dec 9, 2024

Hello,

Describe the inconsistency

There are two options to save the time vector :

  • Option 1 - A vector of size corresponding to the sample time of every data point
  • Option 2 - A vector of size 2 corresponding to the start time and sample spacing

This creates however an issue if the file only has two-time points (very rare but feasible). When parsing the time vector [2 3.5], where the number of samples is 2, then it is not clear if the vector should be interpreted as time <- [2, 3.5] or time <- [2, 5.5]

Additionally, for option 1, the standard doesn't enforce that the vector is sorted (strictly increasing vector) which would make some weirdly formatted files valid.

Current specification

/nirs(i)/data(j)/time

  • Presence: required
  • Type: numeric 1-D array
  • Location: /nirs(i)/data(j)/time

The time variable. This provides the acquisition time of the measurement
relative to the time origin. This will usually be a straight line with slope
equal to the acquisition frequency, but does not need to be equal spacing. For
the special case of equal sample spacing an array of length <2> is allowed
where the first entry is the start time and the
second entry is the sample time spacing in TimeUnit specified in the
metaDataTags. The default time unit is in second ("s"). For example,
a time spacing of 0.2 (s) indicates a sampling rate of 5 Hz.

  • Option 1 - The size of this variable is <number of time points> and
    corresponds to the sample time of every data point
  • Option 2- The size of this variable is <2> and corresponds to the start
    time and sample spacing.

Chunked data is allowed to support real-time streaming of data in this array.

Corrected specification

/nirs(i)/data(j)/time

  • Presence: required
  • Type: numeric 1-D array
  • Location: /nirs(i)/data(j)/time

The time variable. This provides the acquisition time of the measurement relative to the time of origin. This will usually be a straight line with slope equal to the acquisition frequency, but does not need to be equal spacing. For the special case of equal sample spacing an array of length <2> is allowed where the first entry is the start time and the second entry is the sample time spacing in TimeUnit specified in the metaDataTags. The default time unit is in second ("s"). For example, a time spacing of 0.2 (s) indicates a sampling rate of 5 Hz.

  • Option 1 - The size of this variable is <number of time points> and corresponds to the sample time of every data point. The values in this vector should be strictly increasing.
  • Option 2- The size of this variable is <2> and corresponds to the start
    time and sample spacing.

If <number of time points> = 2, then option 2 should be used. In that case, the size of this variable is <2> and corresponds to the start time and sample spacing.
Chunked data is allowed to support real-time streaming of data in this array.

I think this correction allows us to clarify the case where <number of time points> = 2 and makes the following time vector invalid : [ 1 0.1 0.2 2 0.3 0.4 3 ...]

Additional context

I am currently improving the SNIRF support inside NIRSTORM (brainstorm-tools/brainstorm3#756) so it led me to wonder about some edge cases covered by the standard.

@Edouard2laire Edouard2laire added the help wanted Extra attention is needed label Dec 9, 2024
@samuelpowell
Copy link
Collaborator

Thanks @Edouard2laire, I agree that we need to specify that in the case of a length two array the option 2 encoding is assumed.

Whilst I certainly expect that the vast majority of recordings would involve a sorted temporal vector, I don't see why this should be enforced in the specification. All software which reads SNIRF files (indeed, which reads any user input at all) should validate this to ensure that it can be handled appropriately, and trying to save users from themselves through specification is an endless task! Arguments to contrary welcome of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants