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

Issue500 koala recognizer3 #521

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

Issue500 koala recognizer3 #521

wants to merge 37 commits into from

Conversation

towsey
Copy link
Contributor

@towsey towsey commented Nov 3, 2021

Title of PR

What is the purpose of this PR?
To approve a new call recognizer for the male Koala.

Changes

  1. Create a new class, config file and unit test for the recognition of koala calls. Call this the Mark3 recognizer to contrast with the first recognizer (now ten years old) and Mark2 made by Ivan.
  2. Clean up the four generations of classes to detect oscillation events.
    Delete the first experimental version Oscillations2010,cs.
  3. Make two oscillation classes available for generic recognizers. These have different algorithms. Classes are OscillationDetection2012 and OscillationDetection2019. Add extensive documentation and Log comments to these classes so the user can determine what is happening when config parameters are changed.
  4. Refactor the GenericRecognizer code so that the PostProcessing methods are outside the main loop. This enables non-generic call recognizers to process calls in three steps: i) event detection using profiles; ii) filtering of profile events using custom code for the species; and iii) the post-processing steps.
  5. Add extensive commentary to the header of the main Koala recognition class to explain the what and why of the recognizer.

Issues

At the time of submitting this pull request, there is one failing unit test which tests the compression of spectrograms prepared from acoustic indices. I did not write this unit test and I am not sure why it is now failing because it has passed on previous occasions while I have worked in this branch. The error is an out of memory issue because the matrix has a large dimension.

Visual Changes

None

Final Checklist

  • [Anthony ] Assign reviewers if you have permission
  • Assign labels if you have permission
  • Link issues related to PR
  • Link any PRs or issues blocking this PR from being merged
  • Remove/Reduce warnings from edited files
  • [Yes ] Unit tests have been added for changes
  • Ensure CI build is passing

towsey added 30 commits June 23, 2021 14:37
These changes are an unexplained hangover from a previous branch. I am sure they were previously fixed.
Issue #500 The test has not yet been run because cannot get VS2022 to run it yet.
Issue#500 Changed signature of the Oscllation2012 method in order to return an array of decibel values as well as an array of osc score values.
Issue #500  Changed signature of Oscillations2012.Execute():
1: changed order of the arguments to make them consistent across various execute() calls.
2: Inserted two new arguments min and maxOscillationFrequency
3: Convert min and maxOscillFrequency from int to double?
Issue #500 Add white line to top of plot to delineate it from any plot above.
…on2012

Issue #500
Two other methods impacted by change to method signature of Oscillation2012 and change of min/maxOscFreq from int to double?
Issue #500:
Shift Min and MaxOscillationFrequency properties from DctParameters to OscillParameters. These two are properties of an oscillation and not of a DCT.
Issue #500
1: Add the OscillationRate readonly property
2: Refactor the algorithm by which the oscillation rate/periodicity of an OscillationEvent is calculated. It was not working for some events. Also add in a sanity check incase the calculation produces badly erroneous result.
Issue #500
1: Previously changed the return tuple for Oscillations2012.GetComponentsWithOscillations() and had to update the GenericRecognizer accordingly.
Issue #500:
1: Set up new Koala mark3 call recognizer and associated test file.
2: Remove previous test file that was no longer needed.
3: Set up new config file for the test recording.
4: The test recording is temporary - just to get started.
Issue #500 Get the debug spectrogram image working with an overlay of the DCT hits. THis is useful for the Oscillation2012 algorithm.
Issue #500 Make two Oscillation algorithms available for the GenericRecognizer. These are implemented by the Oscillations2012 and Oscillations2019 classes.
Issue #500 Move the ConvertOscillationScores2Events() method because it is used by both the Oscillations2012 and 2019 algorithms.
Issue #500 Add Algorithm property to the Koala config Profile because it is now up to the user to determine which of the two Oscillation algorithms to use.
Issue #500
1: Make adjustments to the start and end of wingbeat events. THis is necessary due to changes in the way the beginning and end of oscillations events are calculated.
2: Make a debug spectrogram image available so user can see what is happening.
Issue #500 Create a new class  to contain DCT methods. Shift across methods from MFCCStuff.cs. Make this a static class but it could/should eventually be used to contain the parameters required to perform the DCT.

A bunch of other methods where affected by this shift of DCT methods.
Issue #500 Deleted some long commented sections from this class. It should eventually be removed. It was the first attempt at writing methods to detect oscillations in spectrograms. It is still being called by some very early recognizers.
Issue #500 Add in some debug comments to make it easier to see what is happening with event creation.
…tions2012 adn Oscillations2019.

Issue #500 Add extensive summary notes on the two oscillation algorithms, Oscillations2012 and Oscillations2019.
Issue #500 Remove the long unmaintained Oscillations2010 class. It s functionality is now replaced by Oscillations2019. The dependency class OscillationRecognizer was also deleted. The remaining dependency, EPR (Event Pattern recognition) class has ben refactored to use the class Oscillations2019. There is still much more work to be done on the EPR class to make it useable in todays AP environment.  The EPR class was originally developed to recognize the Eastern Ground Parrot using recordings from the Sunshine Coast.
Issue #500 ... because it has been replaced by EPR.fs which also has a passing unit test.
Issue #500 Also add comments to the debug log to help user understand how the parameters settings are working.
Issue #500 Adjust unit tests to accommodate changes in the way oscillation boundaries are calculated.
Issue#500 Add information to log about how many harmonic events detected and discarded to make it easier to create a useful recognizer.
issue #500 correct the units of OscillationEvent debug output
Issue #500
 Log10 of 10 = 1.0. No need for the deleted portion.
Issue #500 Remove an unused constant.
Issue #500
I have removed the event post processing code into a separate method. This enables a call to post processing independently of the previous profile event detection.
This is useful for non-generic call recognizers. In particular, it is useful for the Koala3 call recognizer where I wanted to filter the  Koala profile events on features that are specific to the koala bellow.
Issue #500 The calculation of event statistics was refactored in order to be able to access this functionality from within the KoalaMark3 recognizer.
Can now calculate event statistics by supplying the decibel spectrogram rather than the recording.
This required the separation out of the code which calculates the statistics from the code which converts the recording into a spectrogram.
Have added another statistical property, a count of the number of spectral peaks in the spectral profile of the event.
Fix faulty logic in if statements.
I have separated out the PostProcessing steps into a separate method called thus:
results = PostProcessAcousticEvents(configuration, results, segmentStartOffset);
THis is useful for the koala recognizer.

This enables us to catch all the events returned from the Profile method PRIOR to running the Postprocessing steps
So the sequence is:
(1) call the RunProfileEvents() thus:
results = FilterProfileEvents(results, segmentStartOffset);

(2) filter the events using custom methods:
results = FilterProfileEvents(results, segmentStartOffset);

(3) And finally run the postprocessing.
results = PostProcessAcousticEvents(configuration, results, segmentStartOffset);
Issue #500
Add more feed back to user on what is happening during the three processing steps so user can see what effect changes have during the processing.
Three processing steps are:
1: Detection of events using profiles.
2: Filter profile events using custom code for recogniser.
3: Do postprocessing.
Issue #500 Update comments to describe how the recognizer works and why parameter values were chosen etc.
Also comment out the second step (filtering of the profile events prior to post-processing) because it did not help with Koala calls but is likely to be useful for other recognizers.
Issue #500 Edit the config file and construct new unit test for Koala calls. The unit test checks the three levels of returned events.
Issue #500 Comment out code to write debug image. Not required for normal usage.
Copy link
Member

@atruskie atruskie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok right off the bat, this change is rather large and a lot of files are touched for a single species recogniser.

Comment on lines +11 to +22
CommonParameters: &common_parameters
SpeciesName: PhascolarctosCinereus
WindowFunction: HANNING
NoiseReductionType: Standard
BgNoiseThreshold: 0.0


# Each of these profiles will be analyzed
Profiles:
# This profile detections oscillation in the inhale.
KoalaOsc: !OscillationParameters
<<: *common_parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The YAML referencing trick is really only useful when you have to reuse blocks in multiple profile definitions

@@ -1,365 +0,0 @@
// <copyright file="EPR.cs" company="QutEcoacoustics">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing code without a notice of intent is not acceptable. Why has it been removed?

You have no idea if people are using this code or not. On principle alone this change should be reverted

@@ -1,235 +0,0 @@
// <copyright file="OscillationRecogniser.cs" company="QutEcoacoustics">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto as last removed file

@@ -43,7 +43,7 @@ namespace AnalysisPrograms.Production.Arguments
typeof(ContentDescription.BuildModel.Arguments),
typeof(Audio2InputForConvCnn.Arguments),
typeof(DifferenceSpectrogram.Arguments),
typeof(EPR.Arguments),
//typeof(EPR.Arguments),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why comment this if the file is removed?

Comment on lines +159 to +163
var count = combinedResults.NewEvents.Count;
if (count == 0)
{
return combinedResults;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition appears to be useless?

the same value (combinedResult) is returned regardless

Comment on lines +34 to +40
/// Eight of the ten parameters required for these algorithms are the same - just two differences.
/// The eight identical parameters are:
/// (1) MinDuration, (2) MaxDuration, (3) MinHertz, (4) MaxHertz. These constrain the size of the event within the spectrogram.
/// MinHertz and MaxHertz idenfiy the search band. All discovered events will occupy this band.
/// (5) MinOscillationFrequency and (6) MaxOscillationFrequency set the minimum and maximum acceptable oscillation rate for an event.
/// Although these rates are defined as "oscillations per second" the calculations are done using periodicity. Periodicity = 1/OscillationRate.
/// (7) DctDuration and (8) DctThreshold. These parameters determine how the DCT is implemented.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listing these is a great way to cause confusion if their behaviour changes later on - since these names won't be found by a refactor rename (etc) it is super easy for this documentation to get out of sync.

Only list the differences please

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and if you're actually documenting the effect of these parameters, put that documentation in the property's doc string

/// (7) DctDuration and (8) DctThreshold. These parameters determine how the DCT is implemented.
/// DctDuration sets the time span (in seconds) of the DCT. Typically forreliable detection, you would want several oscillations to occur within the DCT duration.
/// DctThreshold (a value in [0,1]) sets the minimum required amplitude for the oscillation to be accepted.
/// The final two parameters are used differently by each algorithm: (9) EventThreshold and (10) DecibelThresholds. ############### SMOOTHING WINDOW
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a major source of confusion, why are they used differently? They're generic parameters

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked, nowhere in this file does it explain it. They should be identical.

profileName);
}

// save a debug image of the spectrogram which includes the HITS overlay.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Comment on lines +62 to +67

//Assert.AreEqual(0.61647, stats.SpectralEnergyDistribution, 1E-4); ######################### PREVIOUS TEST RESULT
Assert.AreEqual(0.5163051699370136, stats.SpectralEnergyDistribution, 1E-4);

//Assert.AreEqual(6011, stats.SpectralCentroid); ######################### PREVIOUS TEST RESULT
Assert.AreEqual(5939, stats.SpectralCentroid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have these changed? Related to the change in SpectrogramDecibels2Power?

@@ -584,7 +584,7 @@ public static double[] Matrix2Array(double[,] matrix)
for (int j = 0; j < cols; j++)
{
//convert decibels to power
double power = Math.Exp(m[i, j] / 10 * Math.Log(10));
double power = Math.Exp(m[i, j] / 10);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok this seems like a subtle but far reaching change. Can you document the reason for the change?

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.

2 participants