Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FixClustGUIAxes #148

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

FixClustGUIAxes #148

wants to merge 7 commits into from

Conversation

gcalongi
Copy link
Contributor

@gcalongi gcalongi commented Jan 8, 2022

Hey Kevin,

I noticed that the axes in the Clustering GUI were based on contour info, not the stored spectrogram in ClusteringData as it seems to internally suppose. I figured there were two ways to address this - either 1) store the boxed image only, which I think was maybe the original intention, or 2) fix how the GUI sees the image. I went with the latter option because it was easier for what I was trying to do (I wanted the axes the same for all the images), but I'm not sure that's what you want, so I just wanted to bring your attention to it.

Happy New Year!

Fixed Clustering GUI axes - were previously based on contour info, not the stored spectrogram in ClusteringData
@ARTUR551
Copy link

Hi,

I was going to ask the same here (I think).
It is very difficult to contextualize and compare USVs in the current Clustering GUI.
gcalongi, how can I apply your update?
i need to make all the axes constant in the Clustering GUI.

USV

Thanks!

A.

@gcalongi
Copy link
Contributor Author

At the moment, my fix is integrated into my master branch, https://github.com/gcalongi/DeepSqueak/tree/master but I try to keep that branch up-to-date with what Dr Coffey is doing, so if he decides to go in a different direction I'm going to amend/retract the commit. In the meantime if you have your own clone, you can cherry-pick that specific commit from my master branch. I have a lot of other branches that will probably keep the commit (e.g. main_GA) but we're doing a lot of other development work in that branch for a specific project, so it may do some other unexpected/undesired things.

@DrCoffey
Copy link
Owner

Hey, sorry it has taken me so long to get around to looking at this, I don't have a lot of time to devote to DeepSqueak at the moment. I will try to get to it in the next couple weeks.

@ARTUR551
Copy link

ARTUR551 commented Feb 1, 2022

Thanks gcalongi!

I tried with the version in your master branch but I get an error when I try to view the clusters.
Here is the text:

Error using tabular/dotParenReference (line 76)
Unrecognized table variable name 'FreqScale'.

Error in clusteringGUI (line 43)
obj.maxfreq = size(ClusteringData.Spectrogram{1},1)*ClusteringData.FreqScale(1);

Error in UnsupervisedClustering_Callback (line 157)
[~, clusterName, rejected, finished, clustAssign] = clusteringGUI(clustAssign, ClusteringData);

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

Error in DeepSqueak (line 29)
gui_mainfcn(gui_State, varargin{:});

Error in
matlab.graphics.internal.figfile.FigFile/read>@(hObject,eventdata)DeepSqueak('UnsupervisedClustering_Callback',hObject,eventdata,guidata(hObject))
Error while evaluating Menu Callback.

I'm not sure if I'm doing something wrong...

Best,
A.

Added the info needed for FixClustGUIAxes to work (FreqScale and TimeScale needed to be added to ClusteringData)
@gcalongi
Copy link
Contributor Author

gcalongi commented Feb 1, 2022

Nope you weren't doing anything wrong that was my bad. I'm working on a couple different branches and forgot that FreqScale and TimeScale were variables that I added at some point. I added the lines that create those variables (commit FixFreqScaleBug) so it should work now if you re-download/re-sync.

The first time you run the clustering, you'll have to use the original detections.mat and recreate the Extracted Contours.mat (to create those columns) if that's the file you were loading.

Let me know if you run into any other errors.

Enabled the ability to load an existing model and continue training.
@gcalongi
Copy link
Contributor Author

gcalongi commented Feb 4, 2022

I added another commit for enabling the ability to build on an existing training model (even when the user chooses to do so). It looked like this capability was almost there but wasn't fully active so I just turned it on. Let me know if there was a reason why you didn't have that active.

@ARTUR551
Copy link

Thanks gcalongi,

I tested your feature and it works nicely.

Do you know if there a way to set (and fix) all the axis of every box? If the boxes remain the same size it is very difficult to contextualize each call, I think changing the size of the box that contains the call gives more information about the call itself. It is definitely more useful.

Best,

A.

@DrCoffey
Copy link
Owner

Hey @gcalongi

I was originally trying to store the boxed image only, but when we switched a to using the power spectral density instead of amplitude for spectrograms, things got lost. It only took me 3 months to fix a single line of code... Sorry I have been absent from DeepSqueak, I had a million other things on my plate.

I may try to set the axes to the size of the largest call (all the same), and still keep only the call in the box, but for now at least the axes are fixed. I pushed the commit and also fixed some lingering spectrogram scaling issues that were really annoying me, so you may want to update to the newest version, and then add your changes back on.

-Kevin

gcalongi added 3 commits June 8, 2022 17:43
Set a unique field UserID in Clustering Data to store the unique CallID from the original detections file (deals with the fact that callID gets renumbered 1:length(CD) after rejected calls are removed)
In clustering GUI, rescaled all spectrograms to the size of the thumbnail, regardless of duration.  This will mean the aspect ratio varies (so user will need to keep an eye on duration) but makes different signals easier to see and contextualize.
@gcalongi
Copy link
Contributor Author

gcalongi commented Jun 9, 2022

Hey @DrCoffey I didn't realize that this open pull request was getting hung up on my end, but it sure explains a lot about how Github has been behaving for the past several months. I've updated to the newest version and added a couple more commits.

The latest commit (ScaleAllToThumbnail) is building off of this particular ongoing thread messing with the Clustering GUI axes. On our end we finally decided to scale all the calls to the thumbnail size regardless of their duration, which means the aspect ratio is going to vary by call but they should be easier to see and contextualize (is this what you meant, @ARTUR551 ?). Went ahead and pushed that in case you'd prefer that as well.

The second-to-last commit (SeparateCall&UserID) was an attempt to share how we've been handling indexing between Calls and ClusteringData that may help with your latest issue indexing Calls after assigning clusters. Not sure that's the direction you want to go, but thought I'd share in case it helps.

@DrCoffey
Copy link
Owner

DrCoffey commented Jun 9, 2022

Hey @gcalongi Thanks for these, I'll take a look when I get a chance. Ya, I'm not sure when it happened but sometime during the recent development phase I screwed up the unsupervised clustering indexing. Unsupervised clustering would kick out rejected calls, and then the call index would be off when updating clusters. I'm overwhelmed with other things at the moment, so the fastest fix was to just include rejected calls in clustering. The easy work around is to just delete the rejected calls with the Remove Rejected Calls tool, and then do clustering.

Added PSD calculation to solve a loading error
@gcalongi
Copy link
Contributor Author

P.S. There was a change to CreateFocusSpectrogram awhile ago that causes an error when you Load Calls because the p variable doesn't exist if make_spectrogram is false, so I added the PSD calculation to the segment when it's false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants