Back to the main page.

Bug 2853 - test_ft_sourcenalysis: try-catch statement prevents proper catching of assertion failures

Status CLOSED FIXED
Reported 2015-02-23 15:29:00 +0100
Modified 2015-09-15 16:07:00 +0200
Product: FieldTrip
Component: test
Version: unspecified
Hardware: PC
Operating System: Windows
Importance: P5 normal
Assigned to: Jan-Mathijs Schoffelen
URL:
Tags:
Depends on:
Blocks:
See also: http://bugzilla.fcdonders.nl/show_bug.cgi?id=2387

Jan-Mathijs Schoffelen - 2015-02-23 15:29:33 +0100

this is occurring around line 1030-1055


Jan-Mathijs Schoffelen - 2015-02-23 15:31:29 +0100

datainfo=ref_datasets; ft_test_sourceanalysis(datainfo(9)); The above would in my expection yield massive problems, due to the recent change in handling of the inside field, because it would be comparing the current output against the output stored on disk, with the insides still indexed.


Jan-Mathijs Schoffelen - 2015-02-23 15:32:51 +0100

I'm taking it for now to look into the issue. I may reassign when the test script starts crashing. Once it starts crashing we need to compare the new output against the old outputs, and ascertain that the only cause of the assertion failure is due to the new inside handling. If so, we need to re-create the benchmark data.


Jan-Mathijs Schoffelen - 2015-02-23 15:42:38 +0100

[jansch@mentat002 fieldtrip]$ svn log test/test_ft_sourceanalysis.m ------------------------------------------------------------------------ r10249 | jansch | 2015-02-23 15:41:24 +0100 (Mon, 23 Feb 2015) | 1 line enhancement - properly error when assertion fails ------------------------------------------------------------------------


Jan-Mathijs Schoffelen - 2015-03-11 09:37:27 +0100

I will test (non-exhaustively) whether the failed assertions are due to the new representation of the inside only + some numeric round off issues in the numeric data. If so, I will replace the current 'latest' data with a fresh version. @Robert: do I need to back up the previous version?


Robert Oostenveld - 2015-03-11 11:23:54 +0100

(In reply to Jan-Mathijs Schoffelen from comment #4) no, I don't consider it needed to back up this.


Jan-Mathijs Schoffelen - 2015-03-17 09:10:08 +0100

apart from the test function failing due to changes in the source structure, there are a bunch of issues which causes specific examples not to be computed due to a crash in ft_sourceanalysis, or one of its depfuns. 1. due to bug 2387, when the geometric units of the input arguments are incompatible. -> I suggest to circumvent this by temporarily equating the units explicitly in test_ft_sourceanalysis with ft_convert_units 2. missing field 'latency' 3. missing field 'frequency' 4. ... other not yet identified. I'll look into 2 and 3 Also, there are some warnings issued due to old style cfg settings: cfg.keepfilter should be cfg.dics.keepfilter etc., causing problems with pedantic checkconfig'ing.


Jan-Mathijs Schoffelen - 2015-03-17 09:16:03 +0100

addition to my previous comment, point 1. The case I am looking into now erroneously sets the units of the grid (with 100 positions expressed in cm) to dm. So this particular case seems to go wrong somewhere else, and is solvable by explicitly stating the units.


Jan-Mathijs Schoffelen - 2015-03-17 09:22:43 +0100

stating the units for the cortical sheets fixes issue 1.


Robert Oostenveld - 2015-03-17 10:05:43 +0100

(In reply to Jan-Mathijs Schoffelen from comment #6) > Also, there are some warnings issued due to old style cfg > settings: cfg.keepfilter should be cfg.dics.keepfilter etc., > causing problems with pedantic checkconfig'ing. I sometimes do an explicit global ft_default ft_default = []; at the start of a test script to ensure that my personal (pedantic) default does not apply, but rather the default (loose).


Jan-Mathijs Schoffelen - 2015-04-03 09:14:17 +0200

[jansch@mentat002 test]$ svn commit -m "enhancement - improved test function and re-computed benchmark results" test_ft_sourceanalysis.m test_ft_sourceanalysis_combinations_allowed.m test_ft_sourceanalysis_combinations_forbidden.m ../ft_sourceanalysis.m Sending ft_sourceanalysis.m Sending test/test_ft_sourceanalysis.m Adding test/test_ft_sourceanalysis_combinations_allowed.m Adding test/test_ft_sourceanalysis_combinations_forbidden.m Transmitting file data .... Committed revision 10316.


Jan-Mathijs Schoffelen - 2015-04-03 09:15:37 +0200

the test function should run through again. To be done: I made a distinction between allowed and forbidden combinations (e.g. doing DICS on timelock data). There exists benchmark data like this, but I suggest that we explicitly throw an error in ft_sourceanalysis when a user wants something like this (I have no idea what's in the benchmark data) and to remove the test data of these combinations.


Jan-Mathijs Schoffelen - 2015-09-15 16:05:46 +0200

Test function seems to be kind of working now, closing.


Jan-Mathijs Schoffelen - 2015-09-15 16:07:00 +0200

Well, actually I am not sure whether the test function actually works, but at least the try catch issue is resolved, so true to its title, this bug has been addressed