sccn / lsl_archived

Multi-modal time-synched data transmission over local network
242 stars 134 forks source link

eeg_load_xdf.m has incorrect event timestamps #287

Open jessicaloke opened 6 years ago

jessicaloke commented 6 years ago

Hello,

I have EEG data recorded with Lab Recorder, OpenBCI and Presentation. However, when load_xdf.m is used, the latency of the events don't appear to be correct. From the experiment, I know that picture stimulus are at least 2 seconds apart but the event times don't reflect this. Attached is my xdf file. Thank you for your help. pp03_a.zip

mgrivich commented 6 years ago

I confirmed that the time between Presentation events vary between a bit less than 1 second and a bit more than 3 (except for the last one, that is more than 8 seconds). The event codes seem to randomly vary between 1 and 15. Do any of these events refer to something other than a screen update? Have you checked with a stop watch to make sure that the screen updates are always more than two seconds apart? If it is still not apparent what is happening, you can zip and send the experiment to me (matthew_grivich at neurobs dot com). Don't send it to the whole list. I'll take a look.

On 4/12/2018 1:39 AM, jessicaloke wrote:

Hello,

I have EEG data recorded with Lab Recorder, OpenBCI and Presentation. However, when load_xdf.m is used, the latency of the events don't appear to be correct. From the experiment, I know that picture stimulus are at least 2 seconds apart but the event times don't reflect this. Attached is my xdf file. Thank you for your help. pp03_a.zip https://github.com/sccn/labstreaminglayer/files/1902104/pp03_a.zip

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/287, or mute the thread https://github.com/notifications/unsubscribe-auth/AFC33W_VC6mXLaod07uLJXUXQzzvizgwks5tnxLTgaJpZM4TRXfu.

jessicaloke commented 6 years ago

The screen refresh rate is set at 60Hz - so, not more than two seconds apart. Is that the issue? I've send you a zip of the experiment.

mgrivich commented 6 years ago

After investigating this issue with the data provided by Jessica, we ascertained that load_xdf.m and eeg_load_xdf.m are not returning the same values for timestamps in an irregular event stream. load_xdf.m is correct and eeg_load_xdf is not. My inspection of eeg_load_xdf reveals numerous errors. I believe that eeg_load_xdf was written to make xdf data friendly for EEGLAB. Is it being used for this purpose still? Does anyone have responsibility for this function?

More detail (Jessica or I can provide the data file upon request): using load_xdf.m: load_xdf('pp01_a.xdf'); Stream 2 is an irregular maker stream data{2}.time_stamps(2) - data{2}.time_stamps(1) = 4.014069011143874 data{2}.time_stamps(3) - data{2}.time_stamps(1) = 4.472975012409734

using eeg_load_xdf.m eeg_load_xdf('pp01_a.xdf'); (data.event(2).latency - data.event(1).latency)/1000 = 0.5011 (data.event(3).latency - data.event(1).latency)/1000 = 0.5584

cboulay commented 6 years ago

Yes, people still use it in EEGLAB.

I'm the last one to work on it so I guess I'm as close as there is to someone 'responsible'. Here is the only commit I made to it, and I guess I did change things in the marker stream timestamps. It looks like I told it to prefer to use effective_srate if it is available, even if the user didn't specify to do so. I'm sorry that I don't remember why I did it, it was over 2 years ago.

Before I beat myself up over this, can you confirm that you and Jessica are using the latest version?

jessicaloke commented 6 years ago

Thanks so much for looking through it. The version that I've used is from xdfimport1.13.


From: Chadwick Boulay notifications@github.com Sent: Tuesday, May 1, 2018 4:33 AM To: sccn/labstreaminglayer Cc: jessicaloke; Author Subject: Re: [sccn/labstreaminglayer] eeg_load_xdf.m has incorrect event timestamps (#287)

Yes, people still use it in EEGLAB.

I'm the last one to work on it so I guess I'm as close as there is to someone 'responsible'. Herehttps://github.com/sccn/xdf/commit/55b4e77f8c1e38af8fe1408917834fc338b9d338 is the only commit I made to it, and I guess I did change things in the marker stream timestamps. It looks like I told it to prefer to use effective_srate if it is available, even if the user didn't specify to do so. I'm sorry that I don't remember why I did it, it was over 2 years ago.

Before I beat myself up over this, can you confirm that you and Jessica are using the latest version?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/sccn/labstreaminglayer/issues/287#issuecomment-385584898, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ASSFZ-8_-mQ7M48HH2nodOTe30oky-_Yks5tt8ldgaJpZM4TRXfu.

cboulay commented 6 years ago

My Matlab license expired and their license server is giving me DNS errors, it'll be at least a couple more days until I have time to test. In the meantime, can you send me a small datafile with this problem? My e-mail is on my github profile, but you can also try attaching directly to a reply here if it's not too big.

jessicaloke commented 6 years ago

Jessica Loke has shared a OneDrive file with you. To view it, click the link below.

https://1drv.ms/u/s!AqfKb3VIed46tRnwohef1IyY5Sm2 [https://r1.res.office365.com/owa/prem/images/dc-generic_20.png]https://1drv.ms/u/s!AqfKb3VIed46tRnwohef1IyY5Sm2

pp01_a 1.xdfhttps://1drv.ms/u/s!AqfKb3VIed46tRnwohef1IyY5Sm2

The datafile is as attached!


From: Chadwick Boulay notifications@github.com Sent: Tuesday, May 1, 2018 5:50 PM To: sccn/labstreaminglayer Cc: jessicaloke; Author Subject: Re: [sccn/labstreaminglayer] eeg_load_xdf.m has incorrect event timestamps (#287)

My Matlab license expired and their license server is giving me DNS errors, it'll be at least a couple more days until I have time to test. In the meantime, can you send me a small datafile with this problem? My e-mail is on my github profile, but you can also try attaching directly to a reply here if it's not too big.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/sccn/labstreaminglayer/issues/287#issuecomment-385705669, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ASSFZ11OwJfNaJAmpeWA5jYYVOIzzWOYks5tuIQogaJpZM4TRXfu.

mgrivich commented 6 years ago

Yes, I checked with a fresh download of load_xdf and eeg_load_xdf.

Some more information: if you use the options to only load the 'Presentation' marker stream, the function errors out.

When you load everything, the 'Presentation' stream is given a sampling rate, but it really shouldn't be, because it is an irregular stream.

I haven't gone though the code in detail to understand exactly how it is supposed to work.

That link that Jessica just shared is corrupt for some reason. Here it is again: https://1drv.ms/u/s!AqfKb3VIed46tRgQaYDYl3zvhQan

On 4/30/2018 7:33 PM, Chadwick Boulay wrote:

Yes, people still use it in EEGLAB.

I'm the last one to work on it so I guess I'm as close as there is to someone 'responsible'. Here https://github.com/sccn/xdf/commit/55b4e77f8c1e38af8fe1408917834fc338b9d338 is the only commit I made to it, and I guess I did change things in the marker stream timestamps. It looks like I told it to prefer to use |effective_srate| if it is available, even if the user didn't specify to do so. I'm sorry that I don't remember why I did it, it was over 2 years ago.

Before I beat myself up over this, can you confirm that you and Jessica are using the latest version?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/287#issuecomment-385584898, or mute the thread https://github.com/notifications/unsubscribe-auth/AFC33fPno1rvGBqZNBppFM1QRhULLVooks5tt8ldgaJpZM4TRXfu.

cboulay commented 6 years ago

eeglab wants the latency field to be in # of samples of the primary time series. So instead of dividing by 1000 above, you need to divide by the sampling rate (125).

I'm starting to remember a little bit about why I made the change I did. The sample# in stream{1} that corresponds to a time_stamp in stream{2} is more accurately calculated (on average) using effective_srate than using nominal_rate. However, seeing as how the xdf file gives us all the timestamps of the primary stream, this can be made even better by searching for the primary stream timestamp nearest the event timestamp and using its index. I'm working on that now.

% Using load_xdf
>> streams = load_xdf(filename);
>> streams{2}.time_stamps(1:3) - streams{2}.time_stamps(1)

ans =

         0    4.0141    4.4730

% Using eeglab plugin
>> EEG = pop_loadxdf(filename , 'streamtype', 'EEG', 'exclude_markerstreams', {});
>> eegl_latency = [EEG.event.latency];
>> [eegl_latency(1:3) - eegl_latency(1)] / 125

ans =

         0    4.0092    4.4675

% Using eeg_load_xdf from github
>> raw = eeg_load_xdf(filename);
>> raw_latency = [raw.event.latency];
>> [raw_latency(1:3) - raw_latency(1)] / 125
ans =

         0    4.0091    4.4675

% Using the primary stream's nominal_srate instead of effective_srate:
ans =

         0    4.0141    4.4730
cboulay commented 6 years ago

@mgrivich , a couple questions for you.

What's more important? That the event 'latency' is correct, or that the event slices the data as close to where the event occurred as possible? I assume the second.

When calling load_xdf with HandleClockSynchronization and HandleJitterRemoval both set to true, should we trust the sample times more than we trust (1:ntimes) / srate ? I assume the answer to this is yes.

Unfortunately, EEG lab does not allow the storage of time stamps. You can only tell it the sample rate. It assumes there are no irregular sampling intervals in an EEG stream. So the samples times that are calculated from the simple 1:npnts / srate calculation that EEGLAB does are different than the source timestamps. This also means that event latencies that are set to be as close to the correct EEG sample as possible will have different timestamps than their origin.

Are the Presentation-only loading errors you're talking about happening in load_xdf.m or eeg_load_xdf.m? eeg_load_xdf.m should only be used with EEGLAB, and that should only be used when there's also a continuous time series because the EEG data structure doesn't make sense without a time series. If you're talking about load_xdf.m, then I guess we should make it work with single marker stream imports too if it doesn't already.

I pushed the latency fix: https://github.com/sccn/xdf/commit/13f50ef057748ca8eacaa5beb4f13df98270a55e Really it's just trading one sort of inaccuracy for another. The previous version could have been the source of your events being off by a bit, but only by a few samples.

mgrivich commented 6 years ago

On 5/2/2018 2:39 PM, Chadwick Boulay wrote:

@mgrivich https://github.com/mgrivich , a couple questions for you.

What's more important? That the event 'latency' is correct, or that the event slices the data as close to where the event occurred as possible? I assume the second.

Yes.

When calling load_xdf with HandleClockSynchronization and HandleJitterRemoval both set to true, should we trust the sample times more than we trust (1:ntimes) / srate ? I assume the answer to this is yes.

Yes.

Unfortunately, EEG lab does not allow the storage of time stamps. You can only tell it the sample rate. It assumes there are no irregular sampling intervals in an EEG stream. So the samples times that are calculated from the simple |1:npnts / srate| calculation that EEGLAB does are different than the source timestamps. This also means that event latencies that are set to be as close to the correct EEG sample as possible will have different timestamps than their origin.

Are the Presentation-only loading errors you're talking about happening in load_xdf.m or eeg_load_xdf.m? eeg_load_xdf.m should only be used with EEGLAB, and that should only be used when there's also a continuous time series because the EEG data structure doesn't make sense without a time series. If you're talking about load_xdf.m, then I guess we should make it work with single marker stream imports too if it doesn't already.

I'm talking about eeg_load_xdf.m It looks like you are shoving a square peg into a round hole. The function needs improved documentation to explain what is going on. And that the output units are samples rather than seconds. And a comment that irregular event streams are converted to the sample timebase. Maybe this is all intuitively obvious to EEGLAB users, but sometimes people (like me) have to understand it without being experienced with EEGLAB.

I pushed the latency fix: 13f50ef057748ca8eacaa5beb4f13df98270a55e Really it's just trading one sort of inaccuracy for another. The previous version could have been the source of your events being off by a bit, but only by a few samples.

I think it's better now. Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/287#issuecomment-386129836, or mute the thread https://github.com/notifications/unsubscribe-auth/AFC33eEdBCwb6uSCr7lvL1hPU_BoSAVEks5tuid9gaJpZM4TRXfu.

soniamgaf commented 6 years ago

Hi,

I tried the new eeg_load_xdf.m function but my results still look weird.

I used LSL to acquired EEG and Bitalino (EDA+ECG+respiratory signals) using a Matlab stimulus that send markers to LSL. When I open the resulting .xdf file in SigViewer the markers seem o.k. but in EEGlab the markers look equally spaced. It is strange because my stimulus has random intervals. I attached the data file that I am testing and the figures from EEGlab and SigViewer.

I am using Matalab2017b and EEGlab14.1.2b (I also tried with EEGlab14.1.1b without success).

I hope that you can help.

Thank you.

Sónia

https://drive.google.com/open?id=16ODOfiFgt4FF5UlknA4hdyYbCNDPhD_h

eeglab

sigviewer

soniamgaf commented 6 years ago

Hi,

I think I solved my own issue. The function load_xdf.m should be used as load_xdf(FileName,'HandleJitterRemoval',false) for markers that are irregular in time. To call it from EEGlab, I changed the function load_xdf.m lines 184-185 at the beginning of the code:

if ~isfield(opts,'HandleJitterRemoval') opts.HandleJitterRemoval = false; end

Now the markers look fine.

mgrivich commented 6 years ago

It would be better to fix it by setting the marker stream to irregular sampling rate as shown in https://github.com/sccn/labstreaminglayer/blob/master/LSL/liblsl-Matlab/examples/SendStringMarkers.m. That way, only that stream will have Jitter Removal deactivated. Jitter Removal improves accuracy for regular streams.

On 5/23/2018 9:14 AM, Sónia Ferreira wrote:

Hi,

I think I solved my own issue. The function load_xdf.m should be used as load_xdf(FileName,'HandleJitterRemoval',false) for markers that are irregular in time. To call it from EEGlab, I changed the function load_xdf.m lines 184-185 at the beginning of the code:

if ~isfield(opts,'HandleJitterRemoval') opts.HandleJitterRemoval = false; end

Now the markers look fine.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/issues/287#issuecomment-391406644, or mute the thread https://github.com/notifications/unsubscribe-auth/AFC33VeRXtUu7DfqIbmEg87IXxR5-3Prks5t1YrcgaJpZM4TRXfu.

soniamgaf commented 6 years ago

Hi,

Thanks a lot! I will try that ;)

soniamgaf commented 6 years ago

Yeah, it's working without changing the 'HandleJitterRemoval' option to false.

Thanks for your help!!!