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.