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 #1955 and #1956 #1959

Merged
merged 5 commits into from
Nov 4, 2021
Merged

Conversation

JohnHalleyGotway
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway commented Nov 4, 2021

Pull Request Testing

This updates develop-ref to get past changes flagged in NB20211103.

  • Describe testing already performed for these changes:

    The differences are caused by caused by 2 PR's for 2 issues:
  1. PR Feature 1905 ens_ctrl #1955 for Issue Add logic to Ensemble-Stat to handle an ensemble control member. #1905 adds new output from ensemble_stat:
    ensemble_stat/ensemble_stat_MASK_SID_CTRL_20120410_120000V_ens.nc 
    ensemble_stat/ensemble_stat_MASK_SID_CTRL_20120410_120000V_orank.txt 
    ensemble_stat/ensemble_stat_MASK_SID_CTRL_20120410_120000V.stat 

It also adds new STDEV output variables to some existing ensemble_stat NetCDF output files:

file2: MET-develop/test_output/ensemble_stat/ensemble_stat_MASK_SID_20120410_120000V_ens.nc
ERROR: NetCDF headers differ:
25a26,37
> 	float APCP_24_A24_ENS_STDEV(lat, lon) ;
file2: MET-develop/test_output/ensemble_stat/ensemble_stat_MASK_SID_CENSOR_20120410_120000V_ens.nc
ERROR: NetCDF headers differ:
25a26,37
> 	float APCP_24_A24_ENS_STDEV(lat, lon) ;
  1. PR Feature 1761 percent thresh #1956 for Issue Support percentile thresholds for frequency bias not equal to 1 (e.g. ==FBIAS0.9) #1761 adds 3 new grid_stat output files:
    perc_thresh/grid_stat_PERC_THRESH_FBIAS_240000L_20120410_000000V_ctc.txt 
    perc_thresh/grid_stat_PERC_THRESH_FBIAS_240000L_20120410_000000V_cts.txt 
    perc_thresh/grid_stat_PERC_THRESH_FBIAS_240000L_20120410_000000V.stat 

These changes are all expected.

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

    None

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

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

    Update reference version, as expected.

  • Please complete this pull request review by [Fill in date].

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)
    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 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 5 commits October 26, 2021 13:08
…tead of only the filename so that the function can tell whether or not the file exists. (#1952)

Co-authored-by: Julie Prestopnik <jpresto@seneca.rap.ucar.edu>
* Per issue #1761 in set_perc() adding code to get FBIAS numeric value, like 1.0 or 0.9, etc. SL

* Per issue #1761: in set_perc(), modified actual percentile calculation at end to use the extracted FBIAS numeric value (float). SL

* Per issue #1761: modified the check on the perc_thresh_freq_bias, just has to be > 0 now. SL

* Per issue #1761: cleaned up code in set_perc(). SL

* Per #1761, updates to Simple_Node::set_perc() to handle variable frequency bias amounts.
Changes include:
- Reverting the formatting of this back to how it originally was in the develop branch. In general, just match the formatting of the existing file, so as the minimize the number of difference lines.
- Add logic to adjust the percentile to be found based on the requested FBIAS value. Multiplying or dividing the percentile by the FBIAS value depends on the inequality type and whether we're bias adjusting the forecast or observation data.
- Adjust the log messages slightly.

Please be aware that I'm not totally confident in these changes. They warrant much more testing. This logic is very, very confusing.

* Per #1761, call compute_percentile() when double-checking the percentile values.

* Per #1761, remove unused variable.

* Per #1761, add warning for percentiles > 100.

* Per #1761. In set_perc(), after testing cleaned up code. SL

* Per issue #1761: adding new config file for testing dynamic FBIAS values. SL

* Per issue #1761: added new unit test for dynamic FBIAS values when running grid_stat. SL

* Per issue #1761, modified FBIAS section to indicated that the user can use dynamic values that are not 1.0. SL

* Update met/docs/Users_Guide/config_options.rst

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update met/docs/Users_Guide/config_options.rst

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update met/docs/Users_Guide/config_options.rst

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update test/config/GridStatConfig_fbias_perc_thresh

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update test/config/GridStatConfig_fbias_perc_thresh

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update test/config/GridStatConfig_fbias_perc_thresh

Co-authored-by: johnhg <johnhg@ucar.edu>

* Update test/config/GridStatConfig_fbias_perc_thresh

Co-authored-by: johnhg <johnhg@ucar.edu>

* Per issue #1761, set nc_pairs_flag = FALSE. SL

Co-authored-by: Seth Linden <linden@kiowa.rap.ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: j-opatz <59586397+j-opatz@users.noreply.github.com>
…ing the logic for computing the ensemble range back to what it was previously. The new version produced very slight differences in the 6-th or 7-th decimal place when compared to previous results. There's not good reason for these changes which were caused by the order of operations in casting from doubles to floats. Reverting back to the old logic prevents diffs for anyone else downstream and is the prudent choice.
@JohnHalleyGotway JohnHalleyGotway merged commit 171b9ed into develop-ref Nov 4, 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.

3 participants