Issue300 fix debug spectrograms #301

towsey · 2020-03-10T00:26:25Z

Original intention was to fix drawing of spectrograms. This included the drawing of acoustic events and the hit matrix overlap. Therefore did some tidying up of the AcousticEvent class. Also set FFT window to HANNING which was originally set as Issue #291.
Wrote unit tests for the drawing of Acoustic events and the merging of overlapping acoustic events.

Work for #300.

Change default window function from Hamming to Hanning. Also add documentation to some of the class properties.

Issue #300 Write one unit test for overlay of sonogram with matrix of score/hits. Remove duplicated method RecognizerBase.WriteSpectrumIndicesFiles() Rewrite method for drawing of event rectangles.

Issue #300 Unit tests for drawing events on sonograms Make unit test for drawing a matrix of scores over a spectrogram. Shift unit test EventStatisticsCalculateTests to a new namespace of tests for acoustic events. Create new class of tests for acoustic events.

Issue #300 The main difficulty here is confusing terminology of temporal fileds and properties. Changed some of these to be more explicit. There appears to remain one more bug - the number of acoustic events in the ae.csv file does not match number that appears in the spectrogram images??

Issue #300 remove DIY test method.

Issue #300 Final debug of merge events. It now works correctly.

Issue #300 Removed unneccesary methods and added documentation. Grouped methods according to functionality.

towsey · 2020-03-10T00:27:35Z

"hit matrix overlap" should read "hit matrix overlay"

atruskie · 2020-03-10T00:30:03Z

src/AnalysisBase/ResultBases/EventBase.cs

+        /// <summary>
+        /// Sets both the Segment start and the Event start wrt to recording.
+        /// </summary>
+        protected void SetSegmentAndEventStartsWrtRecording(TimeSpan segmentStartWrtRecording, double eventStartWrtSegment)


wrt is an unacceptable abbreviation. Remove, please. Use the old names - they are consistently used in the project

If you need to document the purpose of these parameters then add documentation.

atruskie · 2020-03-10T00:30:40Z

src/AnalysisPrograms/Recognizers/Base/WhistleParameters.cs

@@ -30,7 +30,7 @@ public class WhistleParameters : CommonParameters
            double decibelThreshold,
            double minDuration,
            double maxDuration,
-            TimeSpan segmentStartOffset)
+            TimeSpan segmentStartWrtRecording)


Suggested change

TimeSpan segmentStartWrtRecording)

TimeSpan segmentStartOffset)

atruskie · 2020-03-10T00:31:01Z

src/AnalysisPrograms/Recognizers/Base/WhistleParameters.cs

@@ -96,7 +93,7 @@ public class WhistleParameters : CommonParameters
                    decibelThreshold,
                    minDuration,
                    maxDuration,
-                    segmentStartOffset);
+                    segmentStartWrtRecording);


Suggested change

segmentStartWrtRecording);

segmentStartOffset);

atruskie · 2020-03-10T00:31:24Z

src/AnalysisPrograms/Recognizers/Base/WhistleParameters.cs

@@ -110,33 +107,9 @@ public class WhistleParameters : CommonParameters
            } //end for all freq bins

            // combine adjacent acoustic events
-            events = AcousticEvent.CombineOverlappingEvents(events);
+            events = AcousticEvent.CombineOverlappingEvents(events, segmentStartWrtRecording);


atruskie · 2020-03-10T00:32:44Z

src/AudioAnalysisTools/AcousticEvent.cs

+        // double I1MeandB; //mean intensity of pixels in the event prior to noise subtraction
+        // double I1Var;    //,
+        // double I2MeandB; // mean intensity of pixels in the event after Wiener filter, prior to noise subtraction
+        // double I2Var;    //,
+        // private double i3Mean;      // mean intensity of pixels in the event AFTER noise reduciton - USED FOR CLUSTERING
+        // private double i3Var;       // variance of intensity of pixels in the event.
+


These look to be inserted - why are they? Where from? Why not remove them?

Oh, I see, it was removed from further down. I'd suggest removing this completely and instead checking if equivalent statistics can be found in the EventStatistics class

atruskie · 2020-03-10T00:51:58Z

tests/Acoustics.Test/AudioAnalysisTools/AcousticEvents/EventTests.cs

+            var segmentStartOffset = TimeSpan.Zero;
+            double maxPossibleScore = 10.0;
+            var startTime1 = 1.0;
+            var duration1 = 5.0;
+            var minHz1 = 1000;
+            var maxHz1 = 8000;
+            var event1 = new AcousticEvent(segmentStartOffset, startTime1, duration1, minHz1, maxHz1)
+            {
+                Name = "Event1",
+                Score = 1.0,
+                ScoreNormalised = 1.0 / maxPossibleScore,
+            };
+
+            events.Add(event1);
+
+            var startTime2 = 4.5;
+            var duration2 = 2.0;
+            var minHz2 = 1500;
+            var maxHz2 = 6000;
+            var event2 = new AcousticEvent(segmentStartOffset, startTime2, duration2, minHz2, maxHz2)
+            {
+                Name = "Event2",
+                Score = 5.0,
+                ScoreNormalised = 5.0 / maxPossibleScore,
+            };
+            events.Add(event2);
+
+            var startTime3 = 7.0;
+            var duration3 = 2.0;
+            var minHz3 = 1000;
+            var maxHz3 = 8000;
+            var event3 = new AcousticEvent(segmentStartOffset, startTime3, duration3, minHz3, maxHz3)
+            {
+                Name = "Event3",
+                Score = 9.0,
+                ScoreNormalised = 9.0 / maxPossibleScore,
+            };
+            events.Add(event3);


this is a good test (well done) but there's a lot of ceremony here. Inline all the variable declarations that are used to create the acoustic events.

If you want to keep the names as a form of documentation you can use C# named parameters

atruskie · 2020-03-10T00:53:17Z

tests/Acoustics.Test/AudioAnalysisTools/AcousticEvents/EventTests.cs

+            }
+
+            //substituteSonogram.Save("C:\\temp\\image.png");
+            var redPixel1 = new Argb32(110, 10, 30);


Why are you creating pixels with an alpha component here?

atruskie · 2020-03-10T01:18:56Z

tests/Acoustics.Test/AudioAnalysisTools/AcousticEvents/EventTests.cs

+        }
+
+        [TestMethod]
+        public void TestSonogramWithEventsOverlay()


For this test, I'd suggest using the GeneratedImageTest base class I've set up.

Also, the previous comment about ceremony applies here

atruskie · 2020-03-10T01:19:50Z

tests/Acoustics.Test/AudioAnalysisTools/AcousticEvents/EventTests.cs

+                ev.DrawEvent(substituteSonogram, framesPerSecond, freqBinWidth, height);
+            }
+
+            //substituteSonogram.Save("C:\\temp\\image.png");


This is a bad example to leave in. Remove or fix path. See GeneratedImageTests for example output save directory

atruskie · 2020-03-10T01:20:22Z

tests/Acoustics.Test/AudioAnalysisTools/StandardSpectrograms/SonogramTests.cs

@@ -219,5 +221,47 @@ public void TestAnnotatedSonogramWithPlots()
            Assert.AreEqual(1621, image.Width);
            Assert.AreEqual(656, image.Height);
        }
+
+        [TestMethod]
+        public void TestSonogramHitsOverlay()


Use the generated image test class

Issue #300 Most of the changes are to terminology of temporal variables. Also to signatures of test methods.

towsey · 2020-03-10T07:15:59Z

Concerning requested changes:
1: I have reverted the names of temporal variables as requested.
2: Changed signatures of test methods to simplify.
3: Removed methods that were commented out.
4: I have not used the GeneratedImageTest base class because I do not understand how to make use of the examples made available.
5: I created a new name space for Event tests, because it seems sensible to have all test classes of events in the one (more general) name space.

atruskie · 2020-03-10T07:20:50Z

@towsey - thanks.

RE 4.: Set up a call.
RE 5. Tests should roughly mirror source. The event statistics class is in AudioAnalysisTools.EventStatistics in the source so the test should be in Acoustics.Test.AudioAnalysisTools.EventStatistics. What made sense to you is actually less consistent.

towsey · 2020-03-18T00:16:30Z

@atruskie
Concerning point 5, location of the EventStatistics class, my suggestion is that the source should be altered so that both the EventBase class, the AcousticEvent class and the EventStatistics class are all included within the same namespace, e.g. Event namespace.

I will fix the Anti Aliasing issue and then suggest a call concerning point 4.

towsey · 2020-03-19T05:39:54Z

I have not been able to find a solution to the anti-aliasing problem.
Can you send me an example or can we have a call when you are ready?

Issue #300 Fix anti-aliasing problem and then fix unit tests for the drawing of events.

Issue #300 Fixed test of drawing events on spectrograms. Test now uses the new image comparison facility. Set the event name to empty string in order to prevent test failing due to writing of poorly anti-aliased text. Returned font type to the original. Included new expected events image in Fixtures.

@towsey

Adressing comments from #301. @towsey was unwilling to address all comments from code review. This commit actually addresses the feedback. Removes abbreviation `wrt` wherever used in recent changes and instead adds better documentation for parameters. Corrects mistaken comments and assumptions about TimeStart and TimeEnd on AcousticEvent and marks them as obsolete to prevent further misunderstanding. Move EventStatisticsCalculateTests.cs back to its correct folder and namespace. Also renames EventTests.cs to AcousticEventTests.cs (as per our convention) and moves the file to correct namespace and folder.

@towsey

Adressing comments from #301. @towsey was unwilling to address all comments from code review. This commit actually addresses the feedback. Removes abbreviation `wrt` wherever used in recent changes and instead adds better documentation for parameters. Corrects mistaken comments and assumptions about TimeStart and TimeEnd on AcousticEvent and marks them as obsolete to prevent further misunderstanding. Move EventStatisticsCalculateTests.cs back to its correct folder and namespace. Also renames EventTests.cs to AcousticEventTests.cs (as per our convention) and moves the file to correct namespace and folder.

towsey added 7 commits March 4, 2020 10:03

Update SpectrogramSettings.cs

711e06c

Change default window function from Hamming to Hanning. Also add documentation to some of the class properties.

Write one unit test for overlay of hit scores

02947b7

Issue #300 Write one unit test for overlay of sonogram with matrix of score/hits. Remove duplicated method RecognizerBase.WriteSpectrumIndicesFiles() Rewrite method for drawing of event rectangles.

Remove a call to one of my do-it-yourself test methods.

072742f

Issue #300 remove DIY test method.

Update AcousticEvent.cs

fa15eef

Issue #300 Final debug of merge events. It now works correctly.

Update AcousticEvent.cs

ecf3ad0

Issue #300 Removed unneccesary methods and added documentation. Grouped methods according to functionality.

atruskie self-assigned this Mar 10, 2020

atruskie requested changes Mar 10, 2020

View reviewed changes

Changes as requested by Anthony

bca4013

Issue #300 Most of the changes are to terminology of temporal variables. Also to signatures of test methods.

Fix anti-aliasing when drawing event

1d4e524

Issue #300 Fix anti-aliasing problem and then fix unit tests for the drawing of events.

atruskie mentioned this pull request Mar 20, 2020

Fix display of Debug spectrograms and write unit tests #300

Closed

towsey and others added 2 commits March 20, 2020 14:35

Cleaned up event test

e8d593d

atruskie force-pushed the Issue300FixDebugSpectrograms branch from c303f56 to 7804814 Compare March 20, 2020 05:34

atruskie approved these changes Mar 20, 2020

View reviewed changes

atruskie merged commit 2bbb60b into master Mar 20, 2020

atruskie deleted the Issue300FixDebugSpectrograms branch March 20, 2020 05:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue300 fix debug spectrograms #301

Issue300 fix debug spectrograms #301

towsey commented Mar 10, 2020 •

edited by atruskie

Loading

towsey commented Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

atruskie Mar 10, 2020

towsey commented Mar 10, 2020

atruskie commented Mar 10, 2020

towsey commented Mar 18, 2020

towsey commented Mar 19, 2020

	TimeSpan segmentStartWrtRecording)
	TimeSpan segmentStartOffset)

Issue300 fix debug spectrograms #301

Issue300 fix debug spectrograms #301

Conversation

towsey commented Mar 10, 2020 • edited by atruskie Loading

towsey commented Mar 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

towsey commented Mar 10, 2020

atruskie commented Mar 10, 2020

towsey commented Mar 18, 2020

towsey commented Mar 19, 2020

towsey commented Mar 10, 2020 •

edited by atruskie

Loading