Back to the main page.

Bug 3077 - nargin seems to be both a variable and a function in ft_rejectcomponent

Status CLOSED FIXED
Reported 2016-02-22 17:09:00 +0100
Modified 2017-01-17 11:29:41 +0100
Product: FieldTrip
Component: core
Version: unspecified
Hardware: PC
Operating System: Linux
Importance: P5 critical
Assigned to: Robert Oostenveld
URL:
Tags:
Depends on:
Blocks:
See also:

Maarten - 2016-02-22 17:09:01 +0100

I am trying to reject some components using ft_rejectcomponent, and even when I "clear nargin" before executing ft_rejectcomponent(cfg,ica,meg) -the respective structures- I receive this error message: Error: File: ft_rejectcomponent.m Line: 83 Column: 1 "nargin" previously appeared to be used as a function or command, conflicting with its use here as the name of a variable. A possible cause of this error is that you forgot to initialize the variable, or you have initialized it implicitly using load or eval. I believe this is a bug on fieldtrip's side? If I am mistaken I apologize. Best, Maarten


Robert Oostenveld - 2016-02-22 17:14:11 +0100

Hi Maarten, This morning I made some changes related to bug #3075 that affected "nargin" and therefore suspiciously close in time... Looking at the code, I think that my change (not a bug in itself) only revealed a obscure coding pattern that is ft_rejectvisual which is deprecated by MATLAB. Probably the old code was not causing this to be flagged as error yet. line 82: % the data can be passed as input arguments or can be read from disk nargin = 1; nargin = nargin + exist('comp', 'var'); nargin = nargin + exist('data', 'var'); Which MATLAB version are you using (so that I can try to reproduce?


Maarten - 2016-02-22 17:16:14 +0100

I am currently running Matlab R2014a, on our local torque servers.


Robert Oostenveld - 2016-02-22 18:14:13 +0100

I was able to reproduce without problems and have fixed it in my local working copy. mac011> git commit -a [bug3077 edc7d31] FIX - resolve nargin issue reported by Maarten, see http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3077 2 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 test/test_bug3077.m However, I realize that the same incorrect/outdated use of nargin might also be present in other functions. The following are vulnerable mac011> grep -l nargin.*[0-9] *.m ft_artifact_zvalue.m ft_audiovideobrowser.m ft_channelselection.m ft_databrowser.m ft_denoise_pca.m ft_electroderealign.m ft_examplefunction.m ft_layoutplot.m ft_multiplotER.m ft_multiplotTFR.m ft_neighbourplot.m ft_prepare_bemmodel.m ft_prepare_headmodel.m ft_prepare_layout.m ft_prepare_leadfield.m ft_prepare_localspheres.m ft_prepare_neighbours.m ft_prepare_sourcemodel.m ft_qualitycheck.m ft_recodeevent.m ft_rejectartifact.m ft_sensorrealign.m ft_singleplotER.m ft_singleplotTFR.m ft_sourceanalysis.m ft_sourcedescriptives.m ft_sourcemovie.m ft_sourceparcellate.m ft_sourceplot.m ft_statistics_stats.m ft_stratify.m ft_topoplotCC.m ft_topoplotER.m ft_topoplotIC.m ft_topoplotTFR.m ft_volumerealign.m ft_wizard.m


Robert Oostenveld - 2016-02-22 18:57:49 +0100

I went through another bunch of files mac011> git status On branch bug3077 Changes not staged for commit: (use "git add ..." to update what will be committed) (use "git checkout -- ..." to discard changes in working directory) modified: ft_artifact_zvalue.m modified: ft_audiovideobrowser.m modified: ft_databrowser.m modified: ft_electroderealign.m modified: ft_layoutplot.m modified: ft_neighbourplot.m modified: ft_prepare_headmodel.m modified: ft_prepare_layout.m modified: ft_prepare_leadfield.m modified: ft_prepare_neighbours.m modified: ft_qualitycheck.m modified: ft_rejectartifact.m modified: ft_sensorrealign.m modified: ft_sourceanalysis.m modified: ft_sourcemovie.m modified: ft_sourceparcellate.m modified: ft_sourceplot.m modified: ft_stratify.m modified: ft_topoplotCC.m modified: ft_volumerealign.m mac011> git commit -a [bug3077 ce2c080] ENH - consistent handling of nargin, allow data to be read from disk as well. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3077 20 files changed, 130 insertions(+), 78 deletions(-) The ones that I did not consistently deal with yet are the plotting functions (all f them) that use the variable name in the title of the figure. These are not consistent at the moment, and some use cfg.dataname, whereas others don't. </p>

Robert Oostenveld - 2016-02-22 18:58:36 +0100

please note that these changes are not yet in the release version, as I first have to test the changes (since there are so many functions with small changes involved)...


Robert Oostenveld - 2016-02-22 19:03:10 +0100

I created a separate bug for the input variable name handling and nargin/inputfile in the plotting functions, please see bug #3079.


Jan-Mathijs Schoffelen - 2016-02-25 10:57:14 +0100

I am not sure yet, and it could be a coincidence, but after merging of branch 3077 into the master branch, something fishy seems to be going on when doing stuff in matlab2015b as compared to 2014b or earlier. So this is what happens. When working on another bug I tried to run something as simple as ft_freqanalysis, which obscurely crashes in 2015b, and not on 2014b The cause of the error is most likely that rollback_provenance (called in line 231 of ft_freqanalysis) returns cfg.channel = 'all' in 2015b, but the full channel list when called from 2014b (even though before the call to rollback_provenance cfg.channel actually was 'all'). Later on in ft_freqanalysis it is more or less assumed that cfg.channel is a list of labels, apparently relying on rollback_provenance on converting 'all' into this list. As of version 2015b this does not happen anymore. I don't know whether this already existed prior to merging branch bug3077, since I only sporadically used matlab2015b, but this smells like a causal relationship.


Jan-Mathijs Schoffelen - 2016-02-25 10:58:12 +0100

I am not sure yet, and it could be a coincidence, but after merging of branch 3077 into the master branch, something fishy seems to be going on when doing stuff in matlab2015b as compared to 2014b or earlier. So this is what happens. When working on another bug I tried to run something as simple as ft_freqanalysis, which obscurely crashes in 2015b, and not on 2014b The cause of the error is most likely that rollback_provenance (called in line 231 of ft_freqanalysis) returns cfg.channel = 'all' in 2015b, but the full channel list when called from 2014b (even though before the call to rollback_provenance cfg.channel actually was 'all'). Later on in ft_freqanalysis it is more or less assumed that cfg.channel is a list of labels, apparently relying on rollback_provenance on converting 'all' into this list. As of version 2015b this does not happen anymore. I don't know whether this already existed prior to merging branch bug3077, since I only sporadically used matlab2015b, but this smells like a causal relationship.


Jan-Mathijs Schoffelen - 2016-02-25 10:59:24 +0100

or not, because looking closer as the list of changed files, ft_freqanalysis, ft_selectdata, rollback_provenance have not been touched.


Jan-Mathijs Schoffelen - 2016-02-25 11:06:26 +0100

doh, the grepped files are not the ones that have changed... It seems that ft_selectdata returns different data.cfg in 2014b and 2015b, probably due to some preamble/postamble stuff.


Jan-Mathijs Schoffelen - 2016-02-25 11:16:19 +0100

changing cfg = ft_checkconfig(cfg, 'trackconfig', 'off', 'checksize', 'yes'); into cfg = ft_checkconfig(cfg, 'trackconfig', 'report', 'checksize', 'yes'); in ft_postamble_trackconfig seems to fix it. huh?


Robert Oostenveld - 2016-03-10 10:12:58 +0100

(In reply to Jan-Mathijs Schoffelen from comment #11) I am not following this in sufficient detail. Let's discuss in person.


Robert Oostenveld - 2016-06-14 16:21:29 +0200

(In reply to Robert Oostenveld from comment #12) @Maarten, all problems with nargin seem to have been resolved. Matlab 2016a and especially 2016b (see bug 3138) are very strict and therefore the code needed to be cleaned up anyway. @Jan-Mathijs, are there still things to discuss in person? If so, let's meet in person (over a cup of coffee or so).


Jan-Mathijs Schoffelen - 2016-06-14 17:02:46 +0200

(In reply to Robert Oostenveld from comment #13) can we do an 'or so'? :o) I don't have a good recollection of what exactly went wrong when I wrote my earlier comments. I suggest to leave it at this with respect to this bug, and keep eyes and ears open for matlab2015b (which I really should start using).


Maarten - 2016-06-14 17:05:57 +0200

(In reply to Robert Oostenveld from comment #13) Hey Robert, this is probably not a coincidence: it was indeed working fine, but right after your e-mail, I started receiving crashes again, so I guess something was committed that didn't work? Undefined function or variable 'ft_nargin'. Error in ft_preamble_init (line 34) if ft_nargin==0 Error in ft_preamble (line 56) evalin('caller', ['ft_preamble_' cmd]); Error in ft_selectdata (line 89) ft_preamble init % this will reset ft_warning and show the function help if nargin==0 and return an error Error in ft_denoise_synthetic (line 88) data = ft_selectdata(tmpcfg, data); This happened after using ft_denoise_synthetic


Robert Oostenveld - 2016-06-14 18:56:24 +0200

(In reply to Maarten from comment #15) I suspect this to be due to another commit that I did this afternoon. does the problem persist if you close matlab and start it again? I suspect it to be due to a change in one function, and that MATLAB keeps another related function in memory without updating it, although the code should be "recompiled" as well. Please follow up and report at http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3145 thanks Robert


Robert Oostenveld - 2017-01-17 11:29:41 +0100

closed multiple bugs that were resolved some time ago