-
Notifications
You must be signed in to change notification settings - Fork 12
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
Issue #469 rescaling of plot graphs #499
Conversation
Issue #469 Also remove two unused methods from DataTools.cs.
Codecov Report
@@ Coverage Diff @@
## master #499 +/- ##
==========================================
+ Coverage 37.80% 39.39% +1.58%
==========================================
Files 478 482 +4
Lines 68716 67501 -1215
Branches 8002 7872 -130
==========================================
+ Hits 25981 26591 +610
+ Misses 42647 40816 -1831
- Partials 88 94 +6
Continue to review full report at Codecov.
|
Hi @towsey. Good tests but the issue was with the generic recogniser and not the multi recogniser. You probably just need to add the array scaling call in the generic recogniser class. But the way, this is what the |
I have included a loop to rescale plots to fit the accompanying spectrogram. |
var image = SpectrogramTools.GetSonogramPlusCharts(results.Sonogram, results.NewEvents, results.Plots, null); | ||
image.SaveAsPng("C:/temp/image.png"); | ||
var image1 = SpectrogramTools.GetSonogramPlusCharts(results.Sonogram, results.NewEvents, results.Plots, null); | ||
image1.Save(this.TestOutputDirectory + "\\image1.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cannot use backslashes - our tests run on non-windows paltforms.
Use the this.TestOutputDirectory.CombineFile("WhistleSpectrogram.png")
|
||
// create the image for visual confirmation | ||
var concatImage1 = ImageTools.CombineImagesVertically(imageList1); | ||
concatImage1.Save(this.TestOutputDirectory + "ConcatImage1.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not join paths with the +
operator.
Use the SaveTestOutput
function
This comment applies to all usages in this class.
Very soryy but unfortunately, the work involved in fixing this aparnetly small Plot scaling problem got significantly more difficult because I used the OscillationEvent class in the unit test. It turned out that the functionality for much of Oscillation events was not working. So this latest commit is mostly about fixing oscillation events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've suggested one change. When you're ready let me know and I'll merge this in
public void TestOscillationPlusHarmonics() | ||
{ | ||
var config = new GenericRecognizer.GenericRecognizerConfig() | ||
{ | ||
Profiles = new Dictionary<string, object>() | ||
{ | ||
{ | ||
"TestBlob", new BlobParameters() | ||
{ | ||
FrameSize = 1024, | ||
FrameStep = 1024, | ||
MaxHertz = 7200, | ||
MinHertz = 4800, | ||
BgNoiseThreshold = 0.0, | ||
BottomHertzBuffer = 1000, | ||
TopHertzBuffer = 500, | ||
DecibelThresholds = new double?[] { 3.0 }, | ||
} | ||
}, | ||
{ | ||
"LowerBandDTMF_z", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name of the test method and test profile do not match up.
DTMF tones are harmonic not oscillations.
Probably not wrong but it is confusing
Fixes #469 Update MultiRecognizer.cs Issue #469 Minor changes. Except change the FFT window from Hamming to Hanning. Update DataToolsTests.cs Issue #469 Add a new test method to check that the scaling of image plots is working. Update GenericRecognizerTests.cs Issue #469 Remove hard-coded path. Update GenericRecognizer.cs Issue #469 Add in code to rescale the plots to fit the current spectrogram. Changed the signature to SpectrogramTools.GetSonogramPlusCharts() Issue #469 Changed the signature to SpectrogramTools.GetSonogramPlusCharts() so that it also includes the title of the spectrogram. THis affected these 6 methods. Rescaling of the plots happens here. Issue #469 This is ensures that all plots are the same length as the last spectrogram. Update SpectrogramTools.cs Issue #469 Fix the drawing of spectrograms. Update BlobEvent.cs Issue #469 The plot was being added twice for Blob events. Removed one of them. Update OscillationEvent.cs Issue #469 1: Added the Periodicity Property to the Oscillation Event class. 2: Fixed the Draw() method for Oscillation events. The events were not drawing. 3: Added a method to trim Oscillation events using the decibel values as a guide to where the true bounds should be. Update Oscillations2019.cs Issue #469 The main change is due to changing the signature of the ConvertOscillationScores2Events() method Update Oscillations2012.cs Issue #469 1: Delete the GetOscillationFrequency() method 2: Change the way a new Oscillation event is initialised. Update GenericRecognizerTests.cs Issue #469 1: Write a new test method. Its original purpose was to satisfy Anthony's intention of having two profiles with different Windows. This was a way to get the plot scaling fixed. Unfortunately a lot of the functionality to do with the OscillationEvent class was not working. I used this test method to get it working. Update OscillationEvent.cs Issue #469 Rejig the calculation of the oscillation period to make it more accurate. Adjust start and end time test results Issue #469 1: Adjust event expected start and end times for test results involving oscillations. These adjustments brought about by tightening the bounds of an oscillation event. 2: Add periodicity result into the test. The periodicity is the property unique to oscillation events. Update GenericRecognizerTests.cs Issue #469 Change name of test method to match the profiles actually used. Update GenericRecognizerTests.cs Issue #469 Update name of the test method a third time! At present there is no method to convert type AcousticEvent directly to type BlobEvent. Only to type SpectralEvent. Update GenericRecognizerTests.cs Issue #469 Change name of profile to be consistent with Blob profile name.
eb79861
to
7a65f8f
Compare
Rescaling of plots
Rescales plots before rendering them in generic recognisers. Fixes #469
Changes
Wrote a unit test to demonstrate that the rescaling of plots is working.
Issues
None. Except that also change the FFT window to Hanning in MultiRecognizer.cs.
That may or maynot cause a change somewhere.
Final Checklist