Back to the main page.
Bug 2987 - support user specification of analog trigger channels, e.g. in EDF files
| Status | CLOSED FIXED |
| Reported | 2015-10-17 10:56:00 +0200 |
| Modified | 2019-08-10 12:31:36 +0200 |
| Product: | FieldTrip |
| Component: | fileio |
| Version: | unspecified |
| Hardware: | PC |
| Operating System: | Mac OS |
| Importance: | P5 normal |
| Assigned to: | Arjen Stolk |
| URL: | |
| Tags: | |
| Depends on: | |
| Blocks: | |
| See also: |
Robert Oostenveld - 2015-10-17 10:56:36 +0200
Arjen (CC) ran into EDF files with a noisy analog trigger channel. So far ft_read_event only supported EDF+ annotations. He made some suggestions by mail, changes to the code and made the commit r10780. I think this needs to be reviewed for consistency with other code.
Robert Oostenveld - 2015-10-17 11:00:06 +0200
As before in bug 2954, a challenge is how to make the low level functionality available to the end user at the different levels of the fieldtrip interface. We have 1) ft_definetrial 2) ft_trialfun_general or a user's trialfun 3) ft_read_event 4a) private/read_trigger, which calls ft_read_data 4b) or some file format specific code
Robert Oostenveld - 2015-10-17 11:17:27 +0200
at the level of 3 (ft_read_event) there is now (after commit 10780) a complicated clutter due to overlapping functionality between the chanindx, trigindx and trig label options. Probably Arjen did not consider the (previously existing) chanindx and just implemented trigindx+triglabel in parallel. fileio/private/read_trigger uses the option chanindx to specify which channels contain continuously sampled TTL values. One level up, in ft_read_event, the selection of channels that contain the TTL values does not seem to be consistent. In fileio/private/read_trigger the specification that analog TTL values are to be thresholded is not handled consistently. The options denoise, fixneuromag, fixartinis, and threshold all relate to this.
Robert Oostenveld - 2015-10-17 11:19:30 +0200
let me add Jorn to the CC list, since he was probably the last one to have worked on this (and I don't want to bork Artinis support)
Robert Oostenveld - 2015-10-17 11:38:09 +0200
the new peak and through functionality in read_trigger largely replicates the up and down functionality, except that the thresholding is different. Also the naming of peak and through events is not consistent with the naming of other threshold crossing events. Would it not be better to implement a consistent threshold option, either on absolute value, relative value, percentiles, medians, etc? Combined with that thresholding, the up and down option can be used. If desired, ft_read_event can call read_trigger twice (as it does now)
Jörn M. Horschig - 2015-10-19 09:13:02 +0200
Hiho, I implemented the 'fixartinis' flag in the believe that it maintains naming convention, and seemed to be minimally invasive ;) I was just also the one implementing triglabel in 10482 (because for our systems, all analog channels are always appended to the nirs data, so I cannot work with indices. I could use chantype though, but neither of them seemed to be already in place). The 'fixartinis' option is implement to also deal with the noisy AD channels by discretizing the values (i.e. rounding). Threshold detection is not suitable for our system, because legal values can be arbitrary, e.g. proper trigger values can be anything between -4V and 4V. The 'denoise' option just helps for REALLY noisy signals (it neglects changes below 0.8, and our channels are not THAT noisy). The 'threshold' option might do it for us, too (which is supposed to work like 'denoise', but with an arbitrarily set threshold, right?). It'd be also fine to remove the triglabel again (I can communicate settings that will work for our systems to our users, i.e. users would have to manually search for 'ADC*' channels).
Robert Oostenveld - 2015-10-19 09:18:30 +0200
(In reply to Jörn M. Horschig from comment #5) Should we perhaps go for a "discretise" option, supporting various approaches such as thresholding and percentiles?
Jörn M. Horschig - 2015-10-19 09:21:39 +0200
should be generic enough, so it sounds like a good solution to me.
Arjen Stolk - 2015-10-20 21:09:26 +0200
What about building in a flexibility-preserving discretization procedure as follows? The user can define whatever he/she wants to use for calculating a data-dependent threshold on the fly: if ~isempty(threshold) % the trigger channels contain an analog (and hence noisy) TTL signal and should be thresholded if ischar(threshold) % evaluate string (e.g., threshold = 'nanmedian') threshold = eval([threshold '(dat)']); end dat(abs(dat)<threshold) = 0; end
Arjen Stolk - 2015-10-20 22:47:56 +0200
(In reply to Arjen Stolk from comment #8) Actually, made it such that read_trigger.m first digitizes the signal, based on a threshold, or threshold function, prior to find up- and down- transitions (which the code was already doing): if ~isempty(threshold) % the trigger channels contain an analog (and hence noisy) TTL signal and should be thresholded if ischar(threshold) % evaluate string (e.g., threshold = 'nanmedian') threshold = eval([threshold '(dat)']); end % discretize the signal dat(abs(dat)<threshold) = 0; dat(abs(dat)>=threshold) = 1; end Furthermore, ft_read_event.m will only perform this trigger detection on edf data when the detectflank input variable is specified. Otherwise, it will continue reading the annotation channel as before: if ~isempty(detectflank) % parse the trigger channel for events event = read_trigger(filename, 'header', hdr, 'dataformat', dataformat, 'begsample', flt_minsample, 'endsample', flt_maxsample, 'chanindx', chanindx, 'detectflank', detectflank, 'trigshift', trigshift, 'threshold', threshold); elseif issubfield(hdr, 'orig.annotation') && ~isempty(hdr.orig.annotation) % read out the annotation channel for events ...
Arjen Stolk - 2015-10-21 08:06:38 +0200
(In reply to Arjen Stolk from comment #9) An approach to user-specified analog channel thresholding is illustrated here, in the context of EDF data formats: http://www.fieldtriptoolbox.org/getting_started/edf Considering this bug resolved, accordingly. Best regards, Arjen
Jörn M. Horschig - 2015-11-13 14:43:10 +0100
Hi Arjen, just checking back on this. Hmm, your implementation only allows for one trigger value to be sent when using the thresholding. (Due to the dat(hi)=1). So, your 'discretizing' is more like a 'binarization'. Also, it does not work when having multiple AD channels (we have 8) (because dat will be NxM, and threshold will be 1xM, so dat>threshold results in an error). So, I'm afraid I need to stick with something like round(20*dat)/20 ti 'discretize' our AD channels.
Arjen Stolk - 2015-11-13 17:44:47 +0100
(In reply to Jörn M. Horschig from comment #11) Hey Jorn, Sorry I broke your functionality. I didn't know you were using the continuous bit above the threshold. Maybe we should make a 'binarythreshold' option? Ciao! Arjen
Jörn M. Horschig - 2015-11-16 09:19:53 +0100
Hi Arjen, oh, no worries, you didn't break anything for me. It's just that I cannot replace the fixartinis option with what you implemented, which is what I initially had hoped for. I am not sure though if the binarization will break something for anyone else? Anyway, the main purpose of my comment was: I need to keep on having the fixartinis option for our AD channels in.
Arjen Stolk - 2015-11-16 09:32:05 +0100
(In reply to Jörn M. Horschig from comment #13) Hey Jorn, not sure whether I understand what the fixartinis feature is doing, but let me know when you need me considering to change anything. :)
Jörn M. Horschig - 2015-11-16 09:42:51 +0100
Hey Arjen, It's a way of discretizing the signal into fixed steps, e.g. now in steps of 0.1. Through 10*round(..)/10, the signal is allowed to 'fluctuate' by 10*0.5 (i.e. 0.05). Smaller channels will be removed. A dummy example: real data is: [0.01 -0.02 0.04 1.02 -0.04 0.23] I do *10, then I round, then I divide by 10, so * 10 -> [0.1 -0.2 0.4 10.2 -0.4 2.3] round -> [0 0 0 10 0 2] /10 -> [0 0 0 1.0 0 0.2] It's not a beautiful solution, rather a quick and dirty hack. But it's good enough ;)
Jörn M. Horschig - 2015-11-16 09:43:29 +0100
(In reply to Jörn M. Horschig from comment #15) correction: 'fluctuate' by 0.5 / 10
Robert Oostenveld - 2015-11-16 10:35:49 +0100
(In reply to Jörn M. Horschig from comment #16) Just to take a step back. In the case of Arjens EDFs we figured out that certain details regarding trigger detection were too important to leave hidden in a sub-sub-sub-function (ft_definetrial->ft_trialfun_xxx->ft_read_event->read_trigger) where where users would have a more difficult time finding the relevant options. So instead we decided to make ft_trialfun_edf.m. Would the same reasoning not also apply to the data from Artinis? If so, then ft_trialfun_artinis.m would be needed. If the artinis case is so sold and fixed that it would never require scrutiny from a user, then it is appropriate to keep in a sub-sub-sub-function.
Jörn M. Horschig - 2015-11-16 10:40:13 +0100
That might be another option indeed. I am still figuring out how to best handle this for our data (but we only have this one sort of AD channel). I'll check back on this soon.