Back to the main page.

Bug 2160 - ft_multiplotER does not support multiple inputs with different time axes

Status CLOSED FIXED
Reported 2013-05-08 09:56:00 +0200
Modified 2017-10-05 09:09:39 +0200
Product: FieldTrip
Component: plotting
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P3 normal
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Eelke Spaak - 2013-05-08 09:56:24 +0200


Eelke Spaak - 2013-05-08 10:04:05 +0200

added test script Adding test/test_bug2160.m Transmitting file data . Committed revision 8112


Jörn M. Horschig - 2013-08-23 09:47:24 +0200

Hi Eelke, this bug sounds similar to bug 2221, but the error message seems to be different. anyway, I quickly tried to make this work. however, ft_redefinetrial and ft_selectdata do not extend the time axis. any other function that might do what is needed that comes to your mind? also, I'm gonna bother bug 2227 with this.


Jörn M. Horschig - 2013-08-23 09:53:21 +0200

hm, forgot bug 2227, it's more problematic here... so we need to have a common time-axis, which makes us again have all those rounding error problems etc ft_selectdata_new can extract all common parameters - maybe add an option there to extend the time-axis and add nans rather than selecting only the common part of the time-axes (around line 510)?


Diego Lozano Soldevilla - 2013-09-24 14:29:42 +0200

(In reply to comment #3) Discussed at the bugbinge, timelock datasets with different time axes should not be allowed for plotting. I fixed by using /private/checktime throwing an error when mismatch is detected. Sending ft_multiplotER.m Transmitting file data . Committed revision 8491. and I updated test function: @Roemer you can see the try/catch error exception here: Sending test_bug2160.m Transmitting file data . Committed revision 8490.


Diego Lozano Soldevilla - 2013-09-25 11:20:41 +0200

(In reply to comment #4) Wait a minute Jorn. I notice that your changed is creating errors in test_tutorial_coherence and test_tutorial_sensor_analysis. The reason is because ft_multiplotER can handle timelock and freq data and freq type of data can have time field or not. Then ft_multiplotER has to check if time field is present in all input datasets and if all time fields are equal. Could you please send to me a piece of data on where you encountered the problem? I've to go one revision back and check


Jörn M. Horschig - 2013-09-25 12:29:44 +0200

Hi Diego, it's just test_bug2160 that failed, so you can test it with data from that test script


Diego Lozano Soldevilla - 2013-09-25 14:41:02 +0200

(In reply to comment #6) On revision r8536 you assumed that freq data always has time field (lines 226-244) but it's not always the case. Now it's fixed Sending ft_multiplotER.m Transmitting file data . Committed revision 8539.


Diego Lozano Soldevilla - 2013-09-25 15:42:34 +0200

All the following test functions are sensitive to ft_multiplotER dealing with the time axis on freq and timelock datatype test_tutorial_coherence test_tutorial_sensor_analysis test_bug2160 test_bug1357


Diego Lozano Soldevilla - 2013-09-25 15:43:03 +0200

(In reply to comment #8) Robert is going to have a look on this particular issue


Robert Oostenveld - 2013-09-25 16:17:33 +0200

(In reply to comment #9) bugfix - resolved an issue with time, freq and timefreq data as input to ft_multiplotER. Diego started working on this yesterday, Jorn made some changes this morning, but overall it did not fully work yet. Hence I took over and reorganized the recent contributions from Diego and Jorn a bit. It now works with the test script of bug 1357 and of bug2160. I am still testing whether test_tutorial_coherence and test_tutorial_sensor_analysis are also again working (this morning they were not working). mac001> svn commit ft_multiplotER.m private/checkfreq.m test/test_bug2160.m test/test_bug1357.m Sending ft_multiplotER.m Adding private/checkfreq.m Sending test/test_bug1357.m Sending test/test_bug2160.m Transmitting file data .... Committed revision 8540.


Robert Oostenveld - 2013-09-25 16:25:20 +0200

(In reply to comment #10) The script test_bug1976 also started failing due to this. This is because it includes towards the ennd test_bug1049 cd /home/common/matlab/fieldtrip/test test_bug1298 cd /home/common/matlab/fieldtrip/test test_bug1563 cd /home/common/matlab/fieldtrip/test test_bug1599 cd /home/common/matlab/fieldtrip/test test_ft_sourceanalysis cd /home/common/matlab/fieldtrip/test test_ft_sourcemovie cd /home/common/matlab/fieldtrip/test test_ft_timelockanalysis_new cd /home/common/matlab/fieldtrip/test test_historical cd /home/common/matlab/fieldtrip/test test_testbug1563 cd /home/common/matlab/fieldtrip/test I suggest that those lines are removed and that we don't do tests twice. That is followed up at http://bugzilla.fcdonders.nl/show_bug.cgi?id=1976#c6


Robert Oostenveld - 2013-09-25 16:25:58 +0200

(In reply to comment #8) The test_tutorial_coherence runs fine again. Only test_tutorial_sensor_analysis to go...


Diego Lozano Soldevilla - 2013-09-25 18:08:35 +0200

(In reply to comment #12) I updated till revision 8542 and thanks to your changes test_tutorial_sensor_analysis and test_tutorial_coherence are working now (the ones I struggled :) )


Robert Oostenveld - 2013-09-25 18:12:03 +0200

all seems to be fine Note that ft_multiplotER still does not support multiple inputs with different time axes (or freq axes), but at least it gives a useful error. If in the future we do want to have this functionality, it should be done using a single call to ft_selectdata with all input args, as ft_selectdata knows how to trim data to consistent selections along any dimension (also time and freq).


Robert Oostenveld - 2014-02-24 10:56:27 +0100

I closed several bugs at once that all have been resolved for some time. If you disagree, please reopen.


Robert Oostenveld - 2017-10-05 09:09:39 +0200

mac011> git checkout -b bug2160 M ft_multiplotER.m M test/test_bug2160.m Switched to a new branch 'bug2160' Teh change to the plotting functions (now using ft_selectdata) caused the test script to pass at a line where an error was expected. The new time/frequency axis selection is much more powerful, hence more things work now than used to do in 2013. I have updated the test script to reflect the changes. mac011> git commit plotting/ [bug2160 e178395] ENH - avoid "abc = axis" to speed up the functions, this follows from a profiling session 4 files changed, 31 insertions(+), 39 deletions(-) mac011> git commit -a [bug2160 a6626f0] ENH - axis selection is now muchg smarter, which required an update in the test script for http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=2160 2 files changed, 88 insertions(+), 41 deletions(-)