Bugfix 329 negative bcmse

[bikegeek]

Pull Request Testing

• Describe testing already performed for these changes:
  

• created and ran test in METcalcpy/metcalcpy/test/test_sl1l2.py using the data Molly provided that results in negative BCMSE values

• added the test to the GitHub workflow unit_tests.yml

• Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
  

  Hank:
  verify that the METcalcpy/metcalcpy/test/test_sl1l2.py passes with the required Python Packages for METcalcpy

  Molly:
  verify that this version is no longer responsible for erroneous plots

• Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [NA]

• Do these changes include sufficient testing updates? [Yes]

• Will this PR result in changes to the test suite? [Yes]
  
  If yes, describe the new output and/or changes to the existing output:
  

• Please complete this pull request review by before beta3 release.
  

Pull Request Checklist

See the METplus Workflow for details.

• Add any new Python packages to the METplus Components Python Requirements table.
• Review the source issue metadata (required labels, projects, and milestone).
• Complete the PR definition above.
• Ensure the PR title matches the feature or bugfix branch name.
• Define the PR metadata, as permissions allow.
  Select: Reviewer(s)
  Select: Organization level software support Project or Repository level development cycle Project
  Select: Milestone as the version that will include these changes
• After submitting the PR, select the ⚙️ icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
• After the PR is approved, merge your changes. If permissions do not allow this, request that the reviewer do the merge.
• Close the linked issue and delete your feature or bugfix branch from GitHub.

[mollybsmith-noaa]

This does remove the thrown error, but I should point out that the pre-rounding of the components before the final statistic is calculated is going to make our result values slightly off. Are we okay with this?

[bikegeek]

I believe the round-up where implemented by @tatiana Burek ***@***.***> to get consistency with the results produced by the original R scripts.

…

--------------- Minna Win Pronouns: she/her National Center for Atmospheric Research DTC/Research Applications Lab Phone: 303-497-8423 *My work hours may not be your work hours. Please do not feel obliged to reply to this email outside of your normal working hours.* ---------------

On Tue, Jan 2, 2024 at 11:33 AM Molly Smith ***@***.***> wrote: ***@***.**** commented on this pull request. This does remove the thrown error, but I should point out that the pre-rounding of the components before the final statistic is calculated is going to make our result values slightly off. Are we okay with this? — Reply to this email directly, view it on GitHub <#344 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4UJHRKBCY2VKJVPU6V2KDYMRHHLAVCNFSM6AAAAABA7C4TEOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMBQHA2DKMBZHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[bikegeek]

One more thing, I also omitted the rounding of the me and mse and still observed the negative bcmse values, so the rounding isn't the culprit

…

--------------- Minna Win Pronouns: she/her National Center for Atmospheric Research DTC/Research Applications Lab Phone: 303-497-8423 *My work hours may not be your work hours. Please do not feel obliged to reply to this email outside of your normal working hours.* --------------- On Thu, Jan 4, 2024 at 8:05 AM Minna Win-Gildenmeister ***@***.***> wrote:

I believe the round-up where implemented by @tatiana Burek ***@***.***> to get consistency with the results produced by the original R scripts. --------------- Minna Win Pronouns: she/her National Center for Atmospheric Research DTC/Research Applications Lab Phone: 303-497-8423 *My work hours may not be your work hours. Please do not feel obliged to reply to this email outside of your normal working hours.* --------------- On Tue, Jan 2, 2024 at 11:33 AM Molly Smith ***@***.***> wrote: > ***@***.**** commented on this pull request. > > This does remove the thrown error, but I should point out that the > pre-rounding of the components before the final statistic is calculated is > going to make our result values slightly off. Are we okay with this? > > — > Reply to this email directly, view it on GitHub > <#344 (review)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AA4UJHRKBCY2VKJVPU6V2KDYMRHHLAVCNFSM6AAAAABA7C4TEOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMBQHA2DKMBZHE> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

[bikegeek]

Here is Tatiana's explanation of why there is rounding:

Rscript performs rounding with 5 digits for all stats. This was implemented to remove the "noise" from plots and output ASCII files.
We decided to increase the number of digits to 7 in Python because of the MET output where the numbers are rounded to 7 digits.

[mollybsmith-noaa]

Okay, if we're rounding to 5 digits that should be fine. I was expecting less precision, as this was based on an older script.