sccn / roiconnect

ROI connectivity analysis in EEG
39 stars 17 forks source link

Snippets method selection error #53

Closed 99KHayes closed 1 year ago

99KHayes commented 1 year ago

Trying to use snippets options from pop_roi_connect results in errors in the method selection

[This is not possible to link our data or provide screenshots sue to the sensitive nature of the data, but I believe I can point out the problem and solve it without it]


Steps to Reproduce

  1. [download the latest version of roiconnect as of 6/26/23 and add to MATLAB path]
  2. [load a cleaned 64 channel eeg dataset and compute the roiactivity ]
  3. [*attempt to enter the code.
    EEG = pop_roi_connect(EEG, 'morder',20,'naccu',[],'methods', {'CS', 'MIM'}, 'roi_selection', {}, 'snippet', 'on', 'snip_length', 60, 'fcsave_format', 'mean_snips');

    It works as expected when you don't have the snippet options selected*]


Expected behavior:

[I expect it to compute a new eeg variable with the connectivity of MIM and CS to be available to me now to analyze, but I get an error instead
]


Actual behavior:

[*

Brace indexing is not supported for variables of this type.
Error in pop_roi_connect (line 244)
            fc_name = options{2}{fc};  

*]


Proposed Solution:

[* In pop_roi_connect.m line 244

fc_name = options{2}{fc};

it is pulling off the line 157

options = varargin;

instead of the structure with all of the input arguments already formatted which is g that is defined on lines 162-174. So when the code is run with snippet settings its pulling from the options cell array defined in line 157 which is based on whatever order the user writes the inputs in the pop_roi_connect() function which is not how a MATLAB function should work, it isn't supposed to be dependent on order. I propose the following steps to fix this issue:

  1. line 224 currently says
    n_conn_metrics = length(options{2}); % number of connectivity metrics
    it needs to be changed to read n_conn_metrics = length(g.methods); % number of connectivity metrics
  2. line 244 currently says fc_name = options{2}{fc};
    it needs to be changed to fc_name = g.methods{fc};

By updating the code with these simple changes the variation of user function input is accounted for and the actual code for connectivity over snippets can be calculated now. *]


Versions

OS version [multiple Windows 10 systems experienced the error]
MATLAB version ['9.14.0.2239454 (R2023a) Update 1']
EEGLAB version [EEGLAB v2023.0]
nguyen-td commented 1 year ago

Thanks a lot for your report and proposed solution, I implemented it in #51. This should now work as intended. Please reopen the issue if the error still persists.