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

Update develop-ref after #1851. #1856

Merged
merged 15 commits into from
Jul 15, 2021
Merged

Update develop-ref after #1851. #1856

merged 15 commits into from
Jul 15, 2021

Conversation

JohnHalleyGotway
Copy link
Collaborator

Pull Request Testing

Update develop-ref after PR #1851 for issue #1746. This PR adds a new call to unit_wavelet_stat.xml and also modifies the configuration of an existing run.

  • Describe testing already performed for these changes:

    The NB for develop failed on 20210715 with new and modified output.
    kiowa:/d1/projects/MET/MET_regression/develop/NB20210715/test_regression_20210715.log
    Four new files:
ERROR: folder MET-develop-ref/test_output missing 4 files
    wavelet_stat/wavelet_stat_GRIB1_NAM_STAGE4_NO_THRESH_120000L_20120409_120000V_isc.txt 
    wavelet_stat/wavelet_stat_GRIB1_NAM_STAGE4_NO_THRESH_120000L_20120409_120000V.nc 
    wavelet_stat/wavelet_stat_GRIB1_NAM_STAGE4_NO_THRESH_120000L_20120409_120000V.ps 
    wavelet_stat/wavelet_stat_GRIB1_NAM_STAGE4_NO_THRESH_120000L_20120409_120000V.stat 

All modified files are from wavelet_stat:

egrep -i "file1:|ERROR:" test_regression_20210715.log | egrep -B 1 "ERROR:" | egrep file1
file1: MET-develop-ref/test_output/met_test_scripts/wavelet_stat/wavelet_stat_120000L_20050807_120000V_isc.txt
file1: MET-develop-ref/test_output/met_test_scripts/wavelet_stat/wavelet_stat_120000L_20050807_120000V.nc
file1: MET-develop-ref/test_output/met_test_scripts/wavelet_stat/wavelet_stat_120000L_20050807_120000V.ps
file1: MET-develop-ref/test_output/met_test_scripts/wavelet_stat/wavelet_stat_120000L_20050807_120000V.stat
file1: MET-develop-ref/test_output/met_test_scripts/wavelet_stat/wavelet_stat_240000L_20050808_000000V.ps
file1: MET-develop-ref/test_output/python/wavelet_stat_python_120000L_20050807_120000V.ps
file1: MET-develop-ref/test_output/python/wavelet_stat_python_mixed_120000L_20050807_120000V.ps
file1: MET-develop-ref/test_output/wavelet_stat/wavelet_stat_GRIB1_NAM_STAGE4_120000L_20120409_120000V.ps

Note that all the existing PostScript images changed because there was a zero (0) where we should have had a capital letter O instead.

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

    No additional testing needed.

  • Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
    This PR serves as documentation of the change.

  • 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:

    As described above.

  • Please complete this pull request review by [07/06/21].

Pull Request Checklist

See the METplus Workflow for details.

  • 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), Project(s), and Milestone
  • After submitting the PR, select Linked Issues with the original issue number.
  • 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.

jprestop and others added 14 commits June 14, 2021 12:24
Commented out items and added text to description
…to_laton

Bugfix 1817 point2grid latlon to laton
…arsing

Bugfix 1508 tc_gen file list parsing
…_double_type_lat_lon

Bugfix 1838 point2grid supports the double type lat/lon
* Per #1833, changed references to met_help to Discussions.

* Per #1833, fixed typo and removed an unnecessary word
* For issue #1746 modified code to allow users to pass in an empty list (or NA) for forecast and observation thresholds in order to skip applying the threhsolds, but it will still compute stats with the raw fields. SL

* For issue #1746, added new unit test that uses a config file that has empty lists for the forecast and observation thresholds. SL

* For issue #1746 Added some content related to allowing users to set forecast and observation cat thresholds to an empty list in order to skip the binary masking (and consider all grid-points for stats). SL

* Per #1746, cleaning up for consistent indentation.

* Per #1746, cleaning up for consistent indentation.

* Per #1746, add a revision history note, update the plotting range in the postscript output to be [-n,n] where n is the maximum value of the maximum absolute difference and 1.0, and also fix a bug. When the NA threshold comes AFTER a real threshold, the resulting data and difference values were not being updated.

* Per #1746, change the Wavelet-Stat config file values in the the wvlt_plot dictionary by setting plot_min = plot_max = 0.0. That enables the default logic of the tool to take effect. Choose the plotting range of the wavelet plots as [-n,n], where n is the maximum of 1.0 and the maximum absolute difference.

* Per #1746, used apply_fcst_thresh where it should have been apply_obs_thresh.

* Per issue #1746, modified some content related to users being able to skip applying the categorical threhsolds by putting an empty list or NA in the configuration file. SL

* Per issue #1746 Added some warnings if the forecast threshold is set to NA but the observation threshold is not NA (a numeric threshold) and vice versa. SL

* Per #1746, fix a couple of typos and tweak wording in the wavelet-stat chapter.

* Per #1746, loop over each pair of fcst/obs thresholds to check for inconsistent use of the NA threshold type.

* Per #1746, a bit of code cleanup replacing calls to n_elements() with n() to make the code more concise.

* Per #1746, need to reinitialize apply_fcst_thresh and apply_obs_thresh to true inside the loop since the NA threshold can appear anywhere in the list of thresholds.

Co-authored-by: Seth Linden <linden@kiowa.rap.ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
@JohnHalleyGotway JohnHalleyGotway added this to the MET 10.1.0 milestone Jul 15, 2021
@JohnHalleyGotway JohnHalleyGotway linked an issue Jul 15, 2021 that may be closed by this pull request
21 tasks
Copy link
Collaborator

@jprestop jprestop left a comment

Choose a reason for hiding this comment

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

I approve this request.

@JohnHalleyGotway JohnHalleyGotway merged commit 1d7f37d into develop-ref Jul 15, 2021
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.

Make the specification of a binary threshold in Wavelet-Stat optional.
4 participants