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
7 stars 10 forks source link

Is ARTwarp_txt.m still needed? #29

Closed olexandr-konovalov closed 9 months ago

olexandr-konovalov commented 1 year ago

https://github.com/dolphin-acoustics-vip/artwarp/wiki says that the main file to run is ARTwarp_csv.m. Comparing it with ARTwarp_txt.m shows this

artwarp $ diff ARTwarp_csv.m ARTwarp_txt.m
5c5
< global warpFactorLevel vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback2
---
> global vigilance bias learningRate maxNumCategories maxNumIterations sampleInterval resample callback2
7,11c7
< load Default.mat;
< 
< warpFactorLevel = 3; % Set to 3 by default
< 
< addpath(pwd);
---
> load ARTwarp.mat
37a34,38
>     'Callback','ARTwarp_Load_TabDelim_Data', ...
>     'Accelerator','c', ...
>     'Label','Load Frequency Contours (*.txt)', ...
>     'Tag','Fileuimenu4');
> h2 = uimenu('Parent',h1, ...

It seems to me that ARTwarp_csv.m evolved further and introduced warpFactorLevel and also used addpath(pwd); to add the current directory to the path. However, the other file also allows to load .txt files via Load Frequency Contours (*.txt)'. Is the latter something you need to allow? Or was that a temporary workaround?

I would like to suggest that we can delete ARTwarp_txt.m and if need be, add the Load Frequency Contours (*.txt)' block to ARTwarp_csv.m.

A related question is about ARTwarp.mat vs Default.mat. One of them is used by ARTwarp_txt.m and another by ARTwarp_csv.m. They have the same size, but maybe they still differ - can't see easily, as they are binary files. Perhaps also one of them could go away. Also, unless there are strong reasons, I'd prefer ARTwarp.mat as more informative file name.

harvard-tham commented 1 year ago

ARTwarp.mat and Default.mat seems to both declare the same workspace variables but not sure if they do anything else. However Default.mat seems to assign some default values to these variables.

When clicked on 'Run Categerisation' option in the 'Analyse' menu, it will load and display the workspace variables in its input boxes. In my opinion Default.mat would be more useful as it provides default values to run ARTwarp with.

olexandr-konovalov commented 1 year ago

ok, does it makes sense to keep Default.mat and delete ARTwarp.mat then together with ARTwarp_txt.m?

olexandr-konovalov commented 1 year ago

Interestingly, current README says

This version of ARTwarp_txt displays the iteration number and number of whistles analysed in the command window while ARTwarp is running. It saves the results at the end of each iteration.

Anyhow, we are not removing anything for the release of version 1.0.

tatedunbar commented 9 months ago

Currently, the README seems to be inaccurate (or at least misleading). In the release, both ARTwarp_txt and ARTwarp_csv seem to display the same information in the command window while running, and the only differences in ARTwarp_txt are that it allows you to select .txt files for analysis and doesn't provide a default warp level. So the quoted section above should probably be removed or further clarified.

olexandr-konovalov commented 9 months ago

I agree - we now have README in the main branch which still describes things how they were before branching off stable-1 branch from which the release was made. Please feel free to make a PR with updated README - also good exercise for making PRs! If not yet, please submit a new issue about this discrepancy.

olexandr-konovalov commented 9 months ago

@tatedunbar removed old text in #52 - this now may be closed