Convert any sample columns in events files to integers. #356

rwblair · 2023-02-15T19:41:06Z

As per:
https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/05-task-events.html

script I ran from root of the repository:

#!/bin/bash
find  . -name "*_events.tsv" -exec grep -l sample {} \; | while read line;
    do indx=$(head -n1 $line | tr "\t" "\n" | grep -nx sample |  cut -d":" -f1);
    if [[ $indx ]]; then 
        if grep -lzP "\r\n$" $line; then
            rs="\r\n"
        else
            rs="\n"
        fi
        echo "$line at column $indx"
        gawk -v RS=$rs -v col=$indx 'BEGIN { FS=OFS="\t" } { $col=gensub(/([0-9]+)\.[0-9]+/, "\\1",1, $col); print }' $line > tmp && mv tmp $line;

    fi
done

This reverts commit 43cc1da.

effigies · 2023-02-16T20:30:06Z

eeg_face13/sub-006/eeg/sub-006_task-faceFO_events.tsv

@@ -1,300 +1,300 @@
 onset	duration	trial_type	response_time	sample	value


Samples with .5

effigies · 2023-02-16T20:31:04Z

eeg_ds003654s_hed/sub-002/eeg/sub-002_task-FacePerception_run-1_events.tsv

@@ -1,553 +1,553 @@
-onset	duration	sample	event_type	face_type	rep_status	trial	rep_lag	value	stim_file
-0.004	n/a	1.0	setup_right_sym	n/a	n/a	n/a	n/a	3	n/a
-24.2098181818	n/a	6052.4545	show_face_initial	unfamiliar_face	first_show	1	n/a	13	u032.bmp


Samples with lots of extra digits

effigies

It would be good to get feedback from the eeg_face13 contributor, which has non-integer samples (but halves) and ds003654s_* which has the onset column divided by 0.004.

I agree that samples should be integers and this is explicit in the spec.

effigies · 2023-02-16T20:34:42Z

bids-examples/eeg_face13/dataset_description.json

Lines 4 to 7 in 280478e

    
           "Authors": [ 
        
           	"James A. Desjardins", 
        
           	"Sidney J. Segalowitz" 
        
           ],

bids-examples/eeg_ds003654s_hed/dataset_description.json

Lines 6 to 13 in 280478e

    
           "Authors": [ 
        
               "Daniel G. Wakeman", 
        
               "Richard N Henson", 
        
               "Dung Truong (curation)", 
        
               "Kay Robbins (curation)", 
        
               "Scott Makeig (curation)", 
        
               "Arno Delorme (curation)" 
        
           ],

@VisLab you might be a good person to check in with here.

sappelhoff

thanks Ross. I'll take care of mirroring these changes to eeg_matchingpennies, where apparently I used floats instead of integers ... so I'll just cut off the .0 at each sample.

Some further notifications:

for eeg_rishikesh --> @arnodelorme
for fnirs_tapping --> @rob-luke
for eeg_face13 --> @Andesha, I believe

VisLab · 2023-03-15T15:38:46Z

I'll fix these both on the example datasets (all of the hed-related examples have this problem) and on the openNeuro version.

BTW, I noticed that this is referencing bids-examples/eeg_ds003654s_hed/dataset_description.json. However,
we did a correction pull-request which has been previously merged so that the datasets are eeg_ds003645s in bids-examples.

I thought I had caught and corrected all of the 54 --> 45 errors. Where did this one come from @effigies? I'd like to correct that while I am at it. Thx

christinerogers · 2023-03-15T15:46:05Z

@Andesha is this something we could update in the next month perhaps?

(thanks @sappelhoff for bringing my attention to this)

sappelhoff · 2023-03-15T15:47:32Z

I thought I had caught and corrected all of the 54 --> 45 errors. Where did this one come from @effigies? I'd like to correct that while I am at it. Thx

That is simply a git merge conflict. In master, the files are named correctly, see e.g.:

https://github.com/bids-standard/bids-examples/tree/master/eeg_ds003645s_hed

they show up here as wrong because @rwblair's branch is old

Andesha · 2023-03-15T16:24:51Z

Feel free to cut off the decimal values for the samples. That's just a result of down sampling from the original recording of 1024Hz to 512Hz.

effigies · 2023-03-15T16:49:11Z

I reran Ross' script on master and pushed to #362.

@VisLab To confirm, truncating to round down to the nearest int is an acceptable transform?

arnodelorme · 2023-03-15T17:13:07Z

Sample latencies can sometimes be fractional because they come from a different machine that has more resolution than the EEG sampling frequency. This is important for reaction time, for example. Even with EEG sampled at 250 Hz, you need to be able to determine reaction time with 1ms (1000 Hz equivalent sampling rate) precision.

effigies · 2023-03-15T18:00:55Z

sample was introduced in BIDS 1.2.0, saying only:

Column name Description

sample OPTIONAL. Onset of the event according to the sampling scheme of the recorded modality (i.e., referring to the raw data file that the events.tsv file accompanies).

In 1.7.0, it became:

Column name Requirement Level Data type Description

sample OPTIONAL integer Onset of the event according to the sampling scheme of the recorded modality (that is, referring to the raw data file that the events.tsv file accompanies).

The integer type was introduced in bids-standard/bids-specification@034c6a8 (part of bids-standard/bids-specification#827) as part of the schematization. There doesn't appear to have been written discussion on whether sample is an integer or a float. From @arnodelorme's post, it sounds like fractional samples are a desired feature, so it's less an index than an alternative unit of time. From this perspective, the fix is not truncation but relaxing the data type to be number.

There is an open issue about whether it is a 0- or 1-based index here: bids-standard/bids-specification#499. If it is an alternative temporal unit, then I think starting at 0 more sense than starting at 1.

VisLab · 2023-03-15T18:00:57Z

Chris, I think it should be rounded.....

…

On Wed, Mar 15, 2023 at 11:49 AM Chris Markiewicz ***@***.***> wrote: I reran Ross' script on master and pushed to #362 <#362>. @VisLab <https://github.com/VisLab> To confirm, truncating to round down to the nearest int is an acceptable transform? — Reply to this email directly, view it on GitHub <#356 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJCJOQN7M52DOYNGYD6H3LW4HXJPANCNFSM6AAAAAAU5IMT54> . You are receiving this because you were mentioned.Message ID: ***@***.***>

VisLab · 2023-03-15T18:13:17Z

One of the ongoing problems with the BIDS onset, is that there is no precision specification on the onset. I have seen some EEG datasets sampled at say 200 Hz which report event onsets rounded to the nearest int, making them useless in analysis as onsets are in seconds. Sample has the potential to specify the position of an event relative to the underlying data a more accurate precision if you know the sampling rate of the data that the event is tied to. I agree with Arno that it might make sense to leave it as real numbers (or doesn't hurt anything as long as it is clear what it means), especially when the events are generated by a device that is being sampled independently and at a different rate. I would agree that if it is an alternative temporal unit it should start at 0.

…

On Wed, Mar 15, 2023 at 1:01 PM Chris Markiewicz ***@***.***> wrote: sample was introduced in BIDS 1.2.0, saying only: Column name Description sample OPTIONAL. Onset of the event according to the sampling scheme of the recorded modality (i.e., referring to the raw data file that the events.tsv file accompanies). In 1.7.0, it became: Column name Requirement Level Data type Description sample OPTIONAL integer Onset of the event according to the sampling scheme of the recorded modality (that is, referring to the raw data file that the events.tsv file accompanies). The integer type was introduced in bids-standard/bids-specification@ 034c6a8 <bids-standard/bids-specification@034c6a8> (part of bids-standard/bids-specification#827 <bids-standard/bids-specification#827>) as part of the schematization. There doesn't appear to have been written discussion on whether sample is an integer or a float. From @arnodelorme <https://github.com/arnodelorme>'s post, it sounds like fractional samples are a desired feature, so it's less an index than an alternative unit of time. From this perspective, the fix is not truncation but relaxing the data type to be number. There is an open issue about whether it is a 0- or 1-based index here: bids-standard/bids-specification#499 <bids-standard/bids-specification#499>. If it is an alternative temporal unit, then I think starting at 0 more sense than starting at 1. — Reply to this email directly, view it on GitHub <#356 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJCJOXLKETM5PJ5MCPDYUTW4H7WFANCNFSM6AAAAAAU5IMT54> . You are receiving this because you were mentioned.Message ID: ***@***.***>

dorahermes · 2023-03-15T18:32:22Z

+1 on zero-based indexing

effigies · 2023-03-15T18:45:40Z

Thanks for the quick responses, Dora and Kay. I've proposed bids-standard/bids-specification#1441, if any interested parties from this thread would care to comment there.

sappelhoff · 2023-04-11T12:42:52Z

The sample column will be considered an arbitrary column in future BIDS version. As such, its use is dataset-specific and SHOULD be documented in the accompanying JSON files. Thanks for the discussion in #1441, everyone.

rwblair added 3 commits February 15, 2023 11:09

remove trailing '.0' from events.tsv sample column

43cc1da

Revert "remove trailing '.0' from events.tsv sample column"

969a117

This reverts commit 43cc1da.

remove decimal from sample column on events.

2d2dcb0

effigies reviewed Feb 16, 2023

View reviewed changes

sappelhoff reviewed Feb 21, 2023

View reviewed changes

effigies mentioned this pull request Mar 15, 2023

FIX: Convert any sample columns in events files to integers #362

Closed

effigies mentioned this pull request Mar 15, 2023

ENH: Clarify sample column values to be 0-based and possibly non-integer bids-standard/bids-specification#1441

Closed

sappelhoff closed this Apr 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert any sample columns in events files to integers. #356

Convert any sample columns in events files to integers. #356

rwblair commented Feb 15, 2023

effigies Feb 16, 2023

effigies Feb 16, 2023

effigies left a comment

effigies commented Feb 16, 2023

sappelhoff left a comment

VisLab commented Mar 15, 2023

christinerogers commented Mar 15, 2023

sappelhoff commented Mar 15, 2023

Andesha commented Mar 15, 2023

effigies commented Mar 15, 2023

arnodelorme commented Mar 15, 2023

effigies commented Mar 15, 2023

VisLab commented Mar 15, 2023 via email

VisLab commented Mar 15, 2023 via email

dorahermes commented Mar 15, 2023

effigies commented Mar 15, 2023

sappelhoff commented Apr 11, 2023

		@@ -1,300 +1,300 @@
		onset duration trial_type response_time sample value

Convert any sample columns in events files to integers. #356

Convert any sample columns in events files to integers. #356

Conversation

rwblair commented Feb 15, 2023

effigies Feb 16, 2023

Choose a reason for hiding this comment

effigies Feb 16, 2023

Choose a reason for hiding this comment

effigies left a comment

Choose a reason for hiding this comment

effigies commented Feb 16, 2023

sappelhoff left a comment

Choose a reason for hiding this comment

VisLab commented Mar 15, 2023

christinerogers commented Mar 15, 2023

sappelhoff commented Mar 15, 2023

Andesha commented Mar 15, 2023

effigies commented Mar 15, 2023

arnodelorme commented Mar 15, 2023

effigies commented Mar 15, 2023

VisLab commented Mar 15, 2023 via email

VisLab commented Mar 15, 2023 via email

dorahermes commented Mar 15, 2023

effigies commented Mar 15, 2023

sappelhoff commented Apr 11, 2023