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

Setting default FFT window to Hanning #349

Merged
merged 5 commits into from
Aug 17, 2020
Merged

Conversation

towsey
Copy link
Contributor

@towsey towsey commented Jul 30, 2020

Issue #291 Have set the default FFT window to Hanning for calculation of all summary and spectral indices.
However left the Test methods using HAMMING window so do not have to make changes to the output.

Title of PR

What is the purpose of this PR?

Changes

List of changes added as part of this PR. This only needs to include the big changes which reviewers should carefully check.

Issues

Any issues caused by the PR that will need to be addressed in another PR.
These issues may be caused by another branch which will change how this branch functions.

Visual Changes

If the PR has any visual changes to the website, post pictures of the new pages and how they look.
Label any images if more than one is given.

Final Checklist

  • 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
  • Unit tests have been added for changes
  • Ensure CI build is passing

Issue #291 Have set the default FFT window to Hanning for calculation of all summary and spectral indices.
However left the Test methods using HAMMING window so do not have to make changes to the output.
@codecov
Copy link

codecov bot commented Jul 30, 2020

Codecov Report

Merging #349 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #349      +/-   ##
==========================================
+ Coverage   29.30%   29.34%   +0.04%     
==========================================
  Files         475      475              
  Lines       68794    68826      +32     
==========================================
+ Hits        20158    20198      +40     
+ Misses      48636    48628       -8     
Impacted Files Coverage Δ
src/AnalysisPrograms/Create4Sonograms.cs 0.00% <ø> (ø)
src/AnalysisPrograms/SnrAnalysis.cs 0.00% <ø> (ø)
src/TowseyLibrary/FFT.cs 67.69% <ø> (ø)
src/AudioAnalysisTools/DSP/DSP_Frames.cs 48.92% <100.00%> (-0.16%) ⬇️
src/AudioAnalysisTools/ActivityAndCover.cs 93.91% <0.00%> (-0.87%) ⬇️
src/Acoustics.Shared.FSharp/matrix.fs 9.99% <0.00%> (-0.19%) ⬇️
src/TowseyLibrary/MatrixTools.cs 25.58% <0.00%> (+0.19%) ⬆️
...AudioAnalysisTools/Ocillations/Oscillations2014.cs 56.55% <0.00%> (+0.61%) ⬆️
src/AudioAnalysisTools/NeuralNets/BinaryCluster.cs 56.16% <0.00%> (+2.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 115b610...dde213d. Read the comment docs.

@atruskie
Copy link
Member

@towsey this introduced multiple test failures. See https://dev.azure.com/QutEcoacoustics/audio-analysis/_build/results?buildId=2193&view=ms.vss-test-web.build-test-results-tab

While you're fIxing those tests, maybe consider how we could make them less fragile?

@towsey
Copy link
Contributor Author

towsey commented Aug 13, 2020 via email

@atruskie
Copy link
Member

https://dev.azure.com/QutEcoacoustics/audio-analysis/_build/results?buildId=2192&view=ms.vss-test-web.build-test-results-tab

It looks like the build number changed 🤷‍♂️. The link above works

towsey added 4 commits August 14, 2020 10:02
Issue #291 Change from Hamming to Hanning window casuses significant differences in dB values - more than be adjusted for by chaning test sensitivity.
@atruskie atruskie merged commit 0a83479 into master Aug 17, 2020
@atruskie atruskie deleted the Issue291_ChangesToDefaultFFT branch August 17, 2020 00:54
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