Back to the main page.

Bug 3136 - EDF Nsamples in hdr can be noninteger due to calculation using fsample and duration instead of nsamples in file header

Status CLOSED FIXED
Reported 2016-06-06 21:46:00 +0200
Modified 2016-06-07 19:14:09 +0200
Product: FieldTrip
Component: fileio
Version: unspecified
Hardware: All
Operating System: All
Importance: P5 normal
Assigned to: Roemer van der Meij
URL:
Tags:
Depends on:
Blocks:
See also:

Roemer van der Meij - 2016-06-06 21:46:16 +0200

Currently, nsamples is calculated as: hdr.nSamples = EDF.NRec * EDF.Dur * EDF.SampleRate(chansel(1)); (line 275 read_edf.m) However, there can be slight inaccuracies in EDF.Dur (duration of each record), leading to a non-integer nsamples. I propose to use the EDF.SPR field, which holds the number of samples per 'record'. hdr.nSamples = EDF.NRec * EDF.SPR(chansel(1)); Unless something seriously goes wrong in file saving, EDF.SPR should always be an integer. Pull request in a bit.


Roemer van der Meij - 2016-06-06 22:14:26 +0200

I guess this can also be viewed as a philosophy change. Right now, we chose the Duration and the SamplingRate fields to be correct, and value their combination higher than the SPR (samples per record) field (we ignore it). In my example of an error due to non-integer hdr.Nsample, it could be the case, of course, that not the Duration field is incorrect, but the SamplingRate is incorrect. If we use the SPR field instead, we make no assumptions, and let the fields of the data specify how many samples can be read in (SPR field), and the Duration field is left as is. (as a matter of fact, is not used in a particular way at all). Actually, the whole error of non-integer hdr.Nsample seems to be caused by the sampling rate first being calculated out of EDF.SPR / EDF.Dur (eeglab code) and us reversing the process. This seems to cause, in my particular case, a compounding of error: K>> EDF.SPR(1) / EDF.Dur ans = 5000 K>> EDF.NRec * EDF.Dur * (EDF.SPR(1) / EDF.Dur) ans = 1.048572000000000e+06 Since us reversing the calculation (which in some cases compounds error) is unnecessary, I propose the above changes (see pull request for all occurances). Another option is to add rounds everywhere, but what I propose fixes the problem and not the symptom.


Robert Oostenveld - 2016-06-07 09:54:25 +0200

SPR is included in the specification at http://www.edfplus.info/specs/edf.html and not a derived measure. So I am fine with using it.


Robert Oostenveld - 2016-06-07 09:55:15 +0200

I have merged https://github.com/fieldtrip/fieldtrip/pull/171 thanks