[MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements #12003

mscheltienne · 2023-09-20T12:45:53Z

On our site, we tend to send triggers as messages to the eye-link host PC. I have no idea if this is good practice to mark events, but it works 😄 In the ASCII file, you now have lines such as:

3761476	   .	   .	    0.0	  127.0	...
3761477	   .	   .	    0.0	  127.0	...
MSG	3761478 20
3761478	   .	   .	    0.0	  127.0	...
3761479	   .	   .	    0.0	  127.0	...

Where 20 is the str trigger, in this case, a numerical value.
So now, when I parse this file with MNE, I'm getting annotations with an empty name because 20 is interpreted as the offset here

mne-python/mne/io/eyelink/_utils.py

Lines 280 to 294 in d2883d5

    
           # make dataframe for experiment messages 
        
           if raw_extras["event_lines"]["MSG"]: 
        
               msgs = [] 
        
               for tokens in raw_extras["event_lines"]["MSG"]: 
        
                   timestamp = tokens[0] 
        
                   # if offset token exists, it will be the 1st index and is numeric 
        
                   if tokens[1].lstrip("-").replace(".", "", 1).isnumeric(): 
        
                       offset = float(tokens[1]) 
        
                       msg = " ".join(str(x) for x in tokens[2:]) 
        
                   else: 
        
                       # there is no offset token 
        
                       offset = np.nan 
        
                       msg = " ".join(str(x) for x in tokens[1:]) 
        
                   msgs.append([timestamp, offset, msg]) 
        
               df_dict["messages"] = pd.DataFrame(msgs)

In raw_extras["event_lines"]["MSG"], I have:

[['2727960', 'START'], ['3694039', '30'], ['3761478', '20'], ['3764717', '20'], ...]

What is this offset token suppose to be and what is the use-case behind this tokens[1].lstrip("-").replace(".", "", 1).isnumeric() line? @scott-huberty Could you review this change/rationale behind the offset?

scott-huberty · 2023-09-20T13:19:25Z

Thanks @mscheltienne, it makes me feel better that someone else is using this!

On our site, we tend to send triggers as messages to the eye-link host PC. I have no idea if this is good practice to mark events, but it works 😄

This makes sense to me!

you now have lines such as:

3761476	   .	   .	    0.0	  127.0	...
3761477	   .	   .	    0.0	  127.0	...
MSG	3761478 20
3761478	   .	   .	    0.0	  127.0	...
3761479	   .	   .	    0.0	  127.0	...

Awh ok, at least at our lab, the string messages sent to the host PC are usually event messages like:

MSG 174859 -13 DISPLAY_WELCOME_ANIMATION

So in my lab's case, we have DISPLAY_WELCOME_ANIMATION instead of a numerical trigger like 20.

What is this offset token suppose to be

The -13 in the message i shared above is an example of an offset in the message string that is sent to the eyelink host. Since it is not always possible to send messages from the stimulus PC to the eyelink host at the instance they're intended to be interpreted, the message format supports the option of including a positive or negative number that is used to interpret the message time at an offset relative to the actual timestamp.

what is the use-case behind this tokens[1].lstrip("-").replace(".", "", 1).isnumeric()

the offset in the message string can be positive or negative and int-like or float-like. isnumeric returns true for strings that are positive integers, so that (clunky?) code was my way of just dropping any periods or negative signs from the offset and checking if it is numerical (i.e. see code below). Maybe there is a better way of checking whether a string like -17.432 is numerical but I couldn't think of one at the time!

"1".isnumeric() # True
"1.0".isnumeric() # False
"-1".isnumeric() # False
"-1.1".lstrip("-").replace(".", "", 1).isnumeric() # True

Could you review this change/rationale behind the offset?

From a first look it makes sense to me but I'll give it a closer look and test it out ASAP!

mscheltienne · 2023-09-20T13:36:30Z

Numericals are handy because you can send the same message as the trigger value delivered to the MEG or EEG device, without a need for mapping strings to numericals. But anyway, thank you @scott-huberty for the explanation!

Since it is not always possible to send messages from the stimulus PC to the eyelink host at the instance they're intended to be interpreted

Can you detail a bit this part? How do you know you are sending the trigger too "late" and how do you estimate the negative offset that accompanies the message?

MSG 174859 -13 DISPLAY_WELCOME_ANIMATION

Does this mean you use a command similar to:

from pylink import EyeLink

el_tracker = EyeLink("100.1.1.1")
el_tracker.sendMessage("DISPLAY_WELCOME_ANIMATION", offset=-13)  # 13 ms prior?

I now notice this offset argument in the pylink package directly, completely absent from the examples/code snippet I've been using!

scott-huberty · 2023-09-20T13:54:26Z

Numericals are handy because you can send the same message as the trigger value delivered to the MEG or EEG device, without a need for mapping strings to numericals

Makes sense to me!

Can you detail a bit this part? How do you know you are sending the trigger too "late" and how do you estimate the negative offset that accompanies the message?

So my understanding based off my discussions with SR staff, is that if an event like my welcome_animation was sent during a screen refresh on the participant monitor, the welcome_animation will onset after the screen refresh, and this offset in the string message will reflect that. I'm not sure how exactly this offset is estimated, because it is handled internally by the eyelink system. TL;DR, While I usually would use a photo-diode to know exactly when a stimulus onset occurs - it seems like this "offset" mechanism is EyeLink's way of trying to estimate the true onset without needing a photo-diode.

Does this mean you use a command similar to:
from pylink import EyeLink

el_tracker = EyeLink("100.1.1.1")
el_tracker.sendMessage("DISPLAY_WELCOME_ANIMATION", offset=-13)  # 13 ms prior?
I now notice this offset argument in the pylink package directly, completely absent from the examples/code snippet I've been using!

I have to admit that we didn't go the open-source way for our experiment design, we just use Eyelink's proprietary Experiment Builder software : (

My guess is that this offset parameter should be a boolean True / False, as in, include the offset token in the message? Because it is not something I would estimate beforehand, it should be estimated on the fly by Eyelink I think.

mscheltienne · 2023-09-20T14:02:11Z

Note: this post is one message late! Not taking yet into account your last reply.

If I read your message and sendMessage correctly, the format is either MSG TIMESTAMP OFFSET STR or MSG TIMESTAMP STR, but with STR which may or may not contain spaces.

.sendMessage("test")             >>> MSG  TIMESTAMP  test
.sendMessage("test", 2)          >>> MSG  TIMESTAMP  2  test
.sendMessage("test", -2)         >>> MSG  TIMESTAMP  -2  test
.sendMessage("test test")        >>> MSG  TIMESTAMP  test test
.sendMessage("test test", 2)     >>> MSG  TIMESTAMP  2  test
.sendMessage("2")                >>> MSG  TIMESTAMP  2
.sendMessage("2 2")              >>> MSG  TIMESTAMP  2 2  # Problem
.sendMessage("2 2", offset=1)    >>> MSG  TIMESTAMP  1  2 2  # Problem

But you do have an apply_offsets, that we might be able to use in _parse_eyelink_ascii.
If apply_offsets is False, we can consider that the MSG should not include and offset. Thus, MSG can be interpreted as:

>>> MSG  TIMESTAMP  test test      -> desc="test test"
>>> MSG  TIMESTAMP  2  test        -> desc="2 test"
>>> MSG  TIMESTAMP  2              -> desc="2"
>>> MSG  TIMESTAMP  2  2           -> desc="2 2"

Now, if apply_offsets is True, we can still triage like so:

# apply_offsets=True, thus we might have MSG of len(2) with no offset, or MSG of len(3)+ with an offset
if len(token) == 2:
    ts, msg = token
    offset = np.nan
elif len(token) == 3:
    ts = token[0]
    try:
        offset = float(token[1])
    except ValueError:
        offset = np.nan
    msg = " ".join(str(x) for x in tokens[1 if np.isnan(offset) else 2:])
else:
    raise RuntimeError("An other format that we don't know about!")

It's not perfect, because it fails to triage:

.sendMessage("2 2")              >>> MSG  TIMESTAMP  2 2  # Problem

which is now interpreted as an offset of 2; but I don't see how to handle that edge case correctly.

mscheltienne · 2023-09-20T14:13:14Z

OK, so in your case, using the Experiment Builder Software, it's able to estimate this delay between an image flip (at which a stimulus is in theory presented and at which the trigger should be marked) and the time at which you requested the trigger.

With their python binding, the function is:


## Sends the given message to the connected EyeLink tracker. The message will be written to the EyeLink tracker.
#
#@remarks
#This is equivalent to the C API
#\code
#int eyecmd_printf(char *fmt, ...);
#\endcode
#The maximum text length is 130 characters. If the given string has more than 130 characters, the first 130
#characters will be sent and if the send passes this function will return 1. If the text is not truncated,
#0 will be returned on a successful message send.
#
#@param message_text text message to be sent.  It does not support printf() kind of formatting.
#@param offset time offset in millisencond for the message.
#@return
#If there is any problem sending the message, a runtime exception is raised.
def sendMessage(self,message_text,offset=0):
	cut = 0
	if message_text is None or len(message_text) == 0:
		return 0
	message_text=str(message_text)
	#if len(message_text) > 130 :   #bugfix: api-152
		#msg = msg[0:130]
	#	cut = 1
	#print "About to send: cut=",cut,msg
	rv = EyeLinkCBind.sendMessage(self,message_text,offset)
	#print "sent"
	if rv == 0 and cut ==1:
		return 1
	else:
		return rv

It's not pretty, but offset is the time in ms. In theory, I can also estimate in PsychoPy the delay to the next flip, or introduce a callback function on flip to deliver the trigger on the next flip; so now the concept behind the offset makes sense.

I stay on my proposition of the last post, use apply_offsets to further improve triage in _parse_eyelink_ascii; even if it's not perfect and will not parse correctly in 1 case with apply_offsets=True, if the MSG has numerical values with spaces (e.g. 2 2) while the true offset is 0 (corresponds to .sendMessage("2 2")).

scott-huberty · 2023-09-20T14:23:12Z

If I read your message and sendMessage correctly, the format is either MSG TIMESTAMP OFFSET STR or MSG TIMESTAMP STR, but with STR which may or may not contain spaces.

exactly, that's my understanding!

But you do have an apply_offsets, that we might be able to use in _parse_eyelink_ascii.
If apply_offsets is False, we can consider that the MSG should not include and offset.

I think I am following you here, and agree yes, if apply_offsets is False, and there is indeed an offset in the string message, we can just stuff that into the Annotation description i.e. " -13 DISPLAY_WELCOME_ANIMATION"

I stay on my proposition of the last post, use apply_offsets to further improve triage in _parse_eyelink_ascii; even if it's not perfect and will not parse correctly in 1 case with apply_offsets=True, if the MSG has numerical values with spaces (e.g. 2 2) while the true offset is 0 (corresponds to .sendMessage("2 2")).

Sure, +1 from me. Sorry maybe I was going way overkill with trying to parse out and apply offsets to begin with (I think Eric tried to improve my YAGNI radar during my GSOC!). I guess I could have just treated everything after the MSG timestamp as a string message that should be passed to the Annotation description, i.e:

MSG 124400 -13 DISPLAY_WELCOME_ANIMATION -> "-13 DISPLAY_WELCOME_ANIMATION".

mscheltienne · 2023-09-20T14:40:55Z

Let's go with that, honestly I think handling the offset is very important, if you have them. Thanks for the back and forth!

mscheltienne · 2023-09-20T15:00:07Z

Let's see if that passes your test for now, then I'll check if I can modify one of your test or add something else when able.

scott-huberty · 2023-09-21T14:07:39Z

To fix the failing test_multi_block_misc_channels, and I think to make sure the default behavior of read_raw_eyelink stays consistent, you just need to set the default value of apply_offsets to True in the read_raw_eyelink definition

mne-python/mne/io/eyelink/eyelink.py

Lines 22 to 30 in 9de0ef6

    
           def read_raw_eyelink( 
        
               fname, 
        
               *, 
        
               create_annotations=True, 
        
               apply_offsets=False, 
        
               find_overlaps=False, 
        
               overlap_threshold=0.05, 
        
               verbose=None, 
        
           ):

mscheltienne · 2023-09-26T16:08:15Z

If we change the default apply_offsets to True, we now read the description as it was read before with apply_offsets set to True or set to False. But, we now always correct the annotation onset by the offset; which is different from the old behavior when apply_offsets was set to False.

If we keep the default apply_offsets to False, we now parse differently the description (and it might include an offset) but we don't apply any time correction to the annotation onset, retaining the behavior of apply_offsets set to False.

I prefer the later, I don't think we should change the default, and I would consider this PR to be a bugfix.

scott-huberty · 2023-09-26T18:34:58Z

If we change the default apply_offsets to True, we now read the description as it was read before with apply_offsets set to True or set to False. But, we now always correct the annotation onset by the offset; which is different from the old behavior when apply_offsets was set to False.

If we keep the default apply_offsets to False, we now parse differently the description (and it might include an offset) but we don't apply any time correction to the annotation onset, retaining the behavior of apply_offsets set to False.

I prefer the later, I don't think we should change the default, and I would consider this PR to be a bugfix.

Oh yes you are right, and agreed.

…into eyelink

larsoner · 2023-09-30T22:15:18Z

@scott-huberty can you review and approve if you're happy? Then I'll take a cursory look and merge

scott-huberty · 2023-10-02T13:46:21Z

@scott-huberty can you review and approve if you're happy? Then I'll take a cursory look and merge

LGTM. Thanks @mscheltienne !

sappelhoff · 2023-11-01T16:36:14Z

Thanks both! I had this exact problem (https://mne.discourse.group/t/reading-eyelink-data-edf-empty-annotations/7771?u=sappelhoff), and somehow missed this PR before asking my question.

This solves it. And this behavior is also what colleagues at my institute get with a Matlab reader they are using (not sure which one it is) 🙏

…ontains only 2 elements (mne-tools#12003)

mscheltienne added 2 commits September 20, 2023 14:43

don't look for an offset if there are only 2 elements

d221cc5

fix typo

dc92188

mscheltienne requested review from drammock and agramfort as code owners September 20, 2023 12:55

fix typo

948758c

mscheltienne added 3 commits September 20, 2023 16:44

fix doc

0238ef7

triage based on apply_offsets

57fc9af

de-duplicate docstring

6d9fa69

mscheltienne marked this pull request as draft September 20, 2023 18:12

fix tests

dd159c7

mscheltienne added 2 commits September 26, 2023 18:24

add test

0fc960e

Merge remote-tracking branch 'upstream/main' into eyelink

29cbe58

mscheltienne marked this pull request as ready for review September 26, 2023 16:24

mscheltienne changed the title ~~Don't look for an offset in an eyelink message if the message contains only 2 elements~~ [MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements Sep 26, 2023

add entry to changelog

c362c52

mscheltienne requested a review from larsoner as a code owner September 27, 2023 08:07

mscheltienne added 2 commits September 27, 2023 12:36

fix typo

640584a

Merge branch 'eyelink' of https://github.com/mscheltienne/mne-python …

84fb2b0

…into eyelink

larsoner merged commit 9f19bd6 into mne-tools:main Oct 2, 2023
26 checks passed

mscheltienne deleted the eyelink branch October 2, 2023 13:50

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024

[MRG] Don't look for an offset in an eyelink message if the message c…

b917495

…ontains only 2 elements (mne-tools#12003)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements #12003

[MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements #12003

mscheltienne commented Sep 20, 2023 •

edited

Loading

scott-huberty commented Sep 20, 2023 •

edited

Loading

mscheltienne commented Sep 20, 2023

scott-huberty commented Sep 20, 2023 •

edited

Loading

mscheltienne commented Sep 20, 2023

mscheltienne commented Sep 20, 2023 •

edited

Loading

scott-huberty commented Sep 20, 2023 •

edited

Loading

mscheltienne commented Sep 20, 2023

mscheltienne commented Sep 20, 2023

scott-huberty commented Sep 21, 2023

mscheltienne commented Sep 26, 2023

scott-huberty commented Sep 26, 2023

larsoner commented Sep 30, 2023

scott-huberty commented Oct 2, 2023

sappelhoff commented Nov 1, 2023

	# make dataframe for experiment messages
	if raw_extras["event_lines"]["MSG"]:
	msgs = []
	for tokens in raw_extras["event_lines"]["MSG"]:
	timestamp = tokens[0]
	# if offset token exists, it will be the 1st index and is numeric
	if tokens[1].lstrip("-").replace(".", "", 1).isnumeric():
	offset = float(tokens[1])
	msg = " ".join(str(x) for x in tokens[2:])
	else:
	# there is no offset token
	offset = np.nan
	msg = " ".join(str(x) for x in tokens[1:])
	msgs.append([timestamp, offset, msg])
	df_dict["messages"] = pd.DataFrame(msgs)

[MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements #12003

[MRG] Don't look for an offset in an eyelink message if the message contains only 2 elements #12003

Conversation

mscheltienne commented Sep 20, 2023 • edited Loading

scott-huberty commented Sep 20, 2023 • edited Loading

mscheltienne commented Sep 20, 2023

scott-huberty commented Sep 20, 2023 • edited Loading

mscheltienne commented Sep 20, 2023

mscheltienne commented Sep 20, 2023 • edited Loading

scott-huberty commented Sep 20, 2023 • edited Loading

mscheltienne commented Sep 20, 2023

mscheltienne commented Sep 20, 2023

scott-huberty commented Sep 21, 2023

mscheltienne commented Sep 26, 2023

scott-huberty commented Sep 26, 2023

larsoner commented Sep 30, 2023

scott-huberty commented Oct 2, 2023

sappelhoff commented Nov 1, 2023

mscheltienne commented Sep 20, 2023 •

edited

Loading

scott-huberty commented Sep 20, 2023 •

edited

Loading

scott-huberty commented Sep 20, 2023 •

edited

Loading

mscheltienne commented Sep 20, 2023 •

edited

Loading

scott-huberty commented Sep 20, 2023 •

edited

Loading