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.