flatironinstitute / CaImAn-MATLAB

Complete Matlab pipeline for large scale calcium imaging data analysis
GNU General Public License v2.0
252 stars 147 forks source link

Reference to non-existing field in new ROI_GUI.m code #72

Closed HagaiHargil closed 7 years ago

HagaiHargil commented 7 years ago

I'm having some troubles with the new GUI - which seems like a great upgrade generally speaking.

Namely, running run_pipeline.m results in this error when it reaches the ROI_GUI.m part:

Reference to non-existent field 'fitness'.

Error in ROI_GUI>ROI_GUI_OpeningFcn (line 88)
(handles.ROIvars.fitness <= handles.options.min_fitness) & ...

Error in gui_mainfcn (line 220)
    feval(gui_State.gui_OpeningFcn, gui_hFigure, [], guidata(gui_hFigure), varargin{:});

Error in ROI_GUI (line 40)
    [varargout{1:nargout}] = gui_mainfcn(gui_State, varargin{:});

Error in run_pipeline (line 130)
    GUIout = ROI_GUI(A,options,Cn,Coor,keep,ROIvars);

I wasn't able to figure out myself what exactly is missing, but I know that the parent function is local_openfig which fails after calling openFigLegacy. I also noticed that commenting out this line doesn't solve the problem, as the line following it (set(handles.computing_text, 'Visible', 'on');) raises the very same error.

The script was run on MATLAB 2017a installed on a CentOS machine that had been running an earlier version of the run_pipeline script (circa May) successfully. I'll be glad to provide more details if needed.

Thanks.

jkobject commented 7 years ago

Hi !

I am sorry not to have documented it yet. That is why you have this error.

I will very soon push a pipeline that shows how to use it as well. in the mean time here is how it needs to look like (after the first cnmf) :

     % classify components
    [ROIvars.rval_space,ROIvars.rval_time,ROIvars.max_pr,ROIvars.sizeA,keep] = 
    classify_components(Y,A,C,b,f,YrA,options);

    % compute time variation 
    traces = prctfilt(C+YrA,8,1000,100);
    ROIvars.fitness = compute_event_exceptionality(traces,0);
    ROIvars.fitness_delta = compute_event_exceptionality(diff(traces,[],2),0);

    % compute center of mass
    ROIvars.cm = com(A,options.d1,options.d2);

    ROIvars.C = C;
    Coor = plot_contours(A,Cn,options,1); close;
    GUIout = ROI_GUI(A,options,Cn,Coor,keep,ROIvars);   
    options = GUIout{2};
    keep = GUIout{3};
HagaiHargil commented 7 years ago

Thanks for your help, but the bug persists with the same error message.

I'll wait for your pipeline :)

jkobject commented 7 years ago

are you sure you have exactly the same error message ?

jkobject commented 7 years ago

you can have a look at demo_script.m it should work fine.

HagaiHargil commented 7 years ago

I have the same unfortunate error with demo_script.m as well:


Updated spatial components 
27 out of 40 components updated 
39 out of 40 components updated 
40 out of 40 components updated 
27 out of 40 components updated 
39 out of 40 components updated 
40 out of 40 components updated 
Reference to non-existent field 'min_fit_delta'.

Error in ROI_GUI>slider_min_fit_delta_Callback (line 188)
set(handles.min_fit_delta,'String',num2str(handles.options.min_fitness_delta));

Error in gui_mainfcn (line 95)
        feval(varargin{:});

Error in ROI_GUI (line 42)
    gui_mainfcn(gui_State, varargin{:});

Error in
matlab.graphics.internal.figfile.FigFile/read>@(hObject,eventdata)ROI_GUI('slider_min_fit_delta_Callback',hObject,eventdata,guidata(hObject)) 
Error using plot_contours
Too many output arguments.

Error in ROI_GUI>ROI_GUI_OpeningFcn (line 95)
[~,~,im] = plot_contours(handles.A_keep,handles.template,handles.options,0,[],handles.CC,[],find(handles.keep));

Error in gui_mainfcn (line 220)
    feval(gui_State.gui_OpeningFcn, gui_hFigure, [], guidata(gui_hFigure), varargin{:});

Error in ROI_GUI (line 40)
    [varargout{1:nargout}] = gui_mainfcn(gui_State, varargin{:});

Error in demo_script (line 79)
    GUIout = ROI_GUI(A,options,Cn,Coor,keep,ROIvars);
jkobject commented 7 years ago

Well, it is definitely not the same error. min fit delta error is an unknown one ( meaning it is not related to any real problem during the run and to anything in the code) that is not supposed to create you any difficulties. As for the plot contour I think that it means you don't have the one I have pushed in the 1b88ca4 commit. Let me know how it goes.

HagaiHargil commented 7 years ago

You are, of course, completely correct. The errors weren't the same when I used demo_script.m. Sorry about that.

Regarding the handles.min_fit_delta error - I'm not sure why you're saying that it's not supposed to create any difficulties. Throughout the function that crashes slider_min_fit_delta_Callback there are several references to non-existing fields in the handles variable. As I said before, even if I comment out the specific line that fails - the next one will fail as well, and so will the function call [handles.A_keep,handles.keep,handles.cellnum] = replot(handles.A,handles.template,handles.options,handles.template_fig,handles.CC,handles.ROIvars,handles.traceplot,handles); (which comes after), since handles doesn't have any field besides options, and in turn options only has min_fitness_delta.

jkobject commented 7 years ago

Good morning, Have you found the new plot contours function? As for handles, it is given in parameters and possesses all of those fields.

HagaiHargil commented 7 years ago

Hi, I've re-cloned the entire repo following your remarks today, so I'm positive I'm using the new utilities/plot_contours.m function.

jkobject commented 7 years ago

can you look in this function if you have the im argument ?

HagaiHargil commented 7 years ago

Hmm, I do. That being said, I see it in the return signature of the function, but it's not accepted in lines 79 and 121 of demo_script.m. So the variable is redundant, basically.

jkobject commented 7 years ago

yes both lines definitely need something like this [Coor,~,~] = plot_contours(A,Cn,options,1);

HagaiHargil commented 7 years ago

It doesn't seem like gui_mainfcn() can currently really do anything with it.

I'll wait for further instructions\a pull request :) Thanks!

jkobject commented 7 years ago

Well, I have cloned the master branch and run it and it completely works on my side...

HagaiHargil commented 7 years ago

I cloned the repo to my private Windows machine and received the same bug. I tried running it with a different dataset and the bug unfortunately wasn't resolved.

Is your script following the same path that my script does? Meaning: demo_script.m, line 80:

GUIout = ROI_GUI(A,options,Cn,Coor,keep,ROIvars);

To: ROI_GUI.m, line 42: gui_mainfcn(gui_State, varargin{:});

To: gui_mainfcn.m, line 95: feval(varargin{:});

To ROI_GUI>slider_min_fit_delta_Callback, line 188?

Because if it doesn't it might be the key here.

jkobject commented 7 years ago

coming back to my version now, I have the same error as you do. I think I miss something. Matlab can be really shady sometimes.

HagaiHargil commented 7 years ago

I agree. I use it only when I have to, and even then you can still hear me grumping about it all the time :)

I saw the pull request. I'll wait for it to be updated and merged. Thanks a lot for your patience.

jkobject commented 7 years ago

We can agree on that! :) everything should work fine now!

epnev commented 7 years ago

This issue seems to have been resolved so I'm closing it, but feel free to re-open if the situation persists. Thank you both!

HagaiHargil commented 7 years ago

I only got to play with the patch today, and I'm sorry to be the bearer of bad news, but the GUI doesn't appear to be working quite yet. I decided against writing my own pull request, so I'll just list some of the needed fixes that I used, and bugs that still remain:

  1. The first two issues are very similar to the old ones: Reference to non-existent fields in ROIvars. The fix here was simple enough, as it seems like some of the assignments were just missing. The missing fields were fitness, fitness_delta and C. To fix the first two, I added in demo_script.m and run_pipeline.m the following lines (line 64):

    %% classify components
    [ROIvars.rval_space,ROIvars.rval_time,ROIvars.max_pr,ROIvars.sizeA,keep, ...
    ROIvars.fitness, ROIvars.fitness_delta] = classify_components(Y,A,C,b,f,YrA,options);
    ROIvars.C = C;

    and inside classify_components.m I had to uncomment lines 39 and 40. This resolved the issue, showing the GUI in its full glory.

  2. The script doesn't wait for the GUI to close, unlike the behavior in previous versions. I believe some uiwait() or waitfor(handle) lines are missing\commented out.

  3. Some of the buttons of the GUI are not working due to references to non-existent fields. This includes pressing on the image itself (Reference to non-existent field cm and pressing the Simple Mode yellow button (Reference to non-existent field iscomputed).

  4. While the colormap of the main figure inside the GUI is a grayscale colormap (as it should be), all other figures that open just before and after the GUI use the default colormap, which isn't suitable for this type of images.

  5. At least my MATLAB version doens't allow two files with the same name, but with a different extension, to be located in the same directory. So having both demo_script.m and demo_script.mlx raises issues on my machine. A simple renaming is enough.

Thanks again for all your good work.

jkobject commented 7 years ago

Hi !

As I explained in my push, I made pushed my modifications together with the current modification I am working on ( mainly because it was easier for me to work from the current version than to change both mine and an older one ) I think I wrote that the simple mode and the end of the pipeline are not valid yet.

As for the uiwait() it is definitely something that I need to implement.

for matlab raising an error, I do not have this problem on my version 2016. Can yours read mlx files?

For demo pipeline.m the code hasn't been merged because the GUI is not in its final version. so you have to use the pipeline I presented in my comment from 6 days ago :

     % classify components
    [ROIvars.rval_space,ROIvars.rval_time,ROIvars.max_pr,ROIvars.sizeA,keep] = 
    classify_components(Y,A,C,b,f,YrA,options);

    % compute time variation 
    traces = prctfilt(C+YrA,8,1000,100);
    ROIvars.fitness = compute_event_exceptionality(traces,0);
    ROIvars.fitness_delta = compute_event_exceptionality(diff(traces,[],2),0);

    % compute center of mass
    ROIvars.cm = com(A,options.d1,options.d2);

    ROIvars.C = C;
    Coor = plot_contours(A,Cn,options,1); close;
    GUIout = ROI_GUI(A,options,Cn,Coor,keep,ROIvars);   
    options = GUIout{2};
    keep = GUIout{3};

I need to work on something else in the beginning of next week but I should push a final, clearer and documented version at the end of next week or during the week after.

Have a nice weekend !