SNIRF import ignores timeUnit field contrary to specification

[samuelpowell]

Describe the bug

When importing SNIRF data, the timeUnit field is ignored and seconds are silently assumed when computing the sampling frequency of the input data. This is contrary to the specification.

The consequence is that the computed sample rate may be incorrect.

Cause

Rather than demonstrate reproduction, one can observe the cause of this behaviour in the codebase, which makes no reference to timeUnit and computes the sampling rate assuming seconds. From master:

            samplingrate_raw = np.array(dat.get('nirs/data1/time'))
            sampling_rate = 0
            if samplingrate_raw.shape == (2, 1):
                # specified as onset/samplerate
                warn("Onset/sample rate SNIRF not yet supported.")
            else:
                # specified as time points
                fs_diff = np.around(np.diff(samplingrate_raw), decimals=4)
                if len(np.unique(fs_diff)) == 1:
                    # Uniformly sampled data
                    sampling_rate = 1. / np.unique(fs_diff)
                else:
                    # print(np.unique(fs_diff))
                    warn("Non uniform sampled data not supported.")
            if sampling_rate == 0:
                warn("Unable to extract sample rate from SNIRF file.")

Expected results

Either:

• Throw an error if a timeUnit is both present and not equal to 's', or,
• Correctly manipulate time vector according to unit specified.

Actual results

The timeUnit is silently ignored and the resulting sampling frequency is incorrect.

Additional information

Error present on master.

[welcome]

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

[agramfort]

ping @rob-luke

[rob-luke]

@samuelpowell is correct. I will discuss with him today how we can fix this and get a test file.

In general, the SNIRF specification is very large, and MNE only covers a small amount of what can be stored. I have been taking a pragmatic approach and just ensuring we can read data from all the manafacturers. But I will take more care in future PRs to add some defensive programming and throwing some more warnings.

[samuelpowell]

Sample file posted in connected issue. Sample rate in this file is 6.67 Hz (e.g. the time vector is [0, 150] ms).

[samuelpowell]

@rob-luke please note that the sample file I provided included events (stim entries) whose timestamps matches the timeUnit. It transpires this is out of spec and the stim entries always use seconds. I've corrected this and ill provide a smaller sample file shortly, as requested.

[rob-luke]

That's madness from the spec. So your data could be in ms and your stim times would be in s? Crazy.

[samuelpowell]

Indeed so, I will open an issue but this is a breaking change. Note that I'm away until Thursday but will pick up on everything promised when I return.

[samuelpowell]

@rob-luke I attach a pair of smaller sample files. The recordings were made using only three tiles (216 channels) in an eighteen tile cap (10Hz output data rate), include landmarks and sample events, and were converted using version 1.1 of our conversation package.

There are two files included which have some small differences which we have previously discussed.

• mne_sample_18_FU3_29_04_22_20_40_12_standard.snirf : default output format, channels in an arbitrary order, landmarks using LUMO names (Al, Ar, Inion, Nasion, Cz).
• mne_sample_18_FU3_29_04_22_20_40_12_mnestyle.snirf : mne output style, channels reordered to alternate in wavelength, landmarks translated (Al->LPA, Ar -> RPA, Nasion->NASION).

At present, only the latter will load in MNE-NIRS, as the former will result in a ValueError: NIRS channels not ordered correctly. Channels must be ordered as source detector pairs with alternating frequencies: 735 & 850.

The current problems are:

• time unit (this issue)
• channel ordering (no issue opened but we have discussed this and I understand you would like to support arbitrary ordering)
• automatic registration (linked issue)

mne_sample_18_FU3_29_04_22_20_40_12.zip