dolphin-acoustics-vip / artwarp

MATLAB program for automated categorisation of tonal animal sounds
https://github.com/dolphin-acoustics-vip/artwarp/wiki
GNU Lesser General Public License v3.0
6 stars 9 forks source link

Fix ARTwarp window from being unclosable #19

Closed RohanK22 closed 2 years ago

RohanK22 commented 2 years ago

The ARTwarp window fails to close (when we try closing it) due to an error in the callback function that is executed when we try closing the window. This happens because the callback function ('callback1') is an invalid expression that is missing a couple of inverted commas and has an unnecessary end statement!

The Error:

Screenshot 2022-04-01 at 00 10 20

The callback function ('callback1') is loaded from ARTwarp.mat or Default.mat as a String. It is stored as a Matlab variable. Here's the 'callback1': 'global vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback1 callback2; shh=get(0,'ShowHiddenHandles'); set(0,'ShowHiddenHandles','on'); delete(get(0,'CurrentFigure')); set(0,'ShowHiddenHandles',shh); filename = which('ARTwarp.mat'); filename = sprintf('\'%s\'',filename); eval(['save ' filename ' vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback1 callback2']); end'

The fact that a function is stored as a string makes it very hard to debug. Ideally, these functions should be converted into MATLAB scripts or functions but I've chosen to keep it this way since changing them would mean changing the config files (.mat files) of ARTwarp leading to possible inconsistencies with the version being used already.

Here's the fix

Change filename = sprintf('\'%s\'',filename); to filename = sprintf('\''%s\''',filename); and remove the end statement from the function.

Here's the fixed callback function:

'global vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback1 callback2; shh=get(0,'ShowHiddenHandles'); set(0,'ShowHiddenHandles','on'); delete(get(0,'CurrentFigure')); set(0,'ShowHiddenHandles',shh); filename = which('ARTwarp.mat'); filename = sprintf('\''%s\''',filename); eval(['save ' filename ' vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback1 callback2']);'

So this PR changes the value of 'callback1' in both ARTwarp.mat and Default.mat to be the correct function that closes the window successfully when called. It is very hard to check if this works by just inspecting the files since the .mat files can't be read easily. The best way would be to test them locally if they work.

Note: Both ARTwarp.mat and Default.mat have the same contents. ARTwarp currently uses Default.mat. We can eliminate one of the files.

olexandr-konovalov commented 2 years ago

Good! is there a way to put this function into .m file and load from there, so that we don't keep it in the binary file?

olexandr-konovalov commented 2 years ago

P.S. The last line of the description is incomplete?

Note: Both ARTwarp.mat and Default.mat have the same contents. ARTwarp currently uses

RohanK22 commented 2 years ago

Good! is there a way to put this function into .m file and load from there, so that we don't keep it in the binary file?

@alex-konovalov, yes I think we could move the functions into a separate .m file. Even if I did that I would have to remove the callback functions from the mat files thus changing them. This feature needed to be fixed very quickly so I didn't think of doing that. But I think that's something that must be done. For now, it would be great if we could approve this since this bug has been annoying users for a long time.