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

Feature 1918 std climo #2061

Merged
merged 13 commits into from
Feb 23, 2022
Merged

Feature 1918 std climo #2061

merged 13 commits into from
Feb 23, 2022

Conversation

JohnHalleyGotway
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway commented Feb 20, 2022

Expected Differences

  • Do these changes introduce new tools, command line arguments, or configuration file options? [Yes]

    If yes, please describe:

Adds "normalize" option to the Gen-Ens-Prod config file.

  • Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]

    If yes, please describe:

Pull Request Testing

  • Describe testing already performed for these changes:

    Tested manually and also added a new call in unit_gen_ens_prod.xml to exercise all available options for the normalize config option.

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

  • Review the code changes.

  • Review the documentation updates in the gen_ens_prod chapter.

  • Inspect the new unit test gen_ens_prod NetCDF output file to make sure the outputs look good.

  • Consider this design choice:

    • For normalize = FCST_STD_ANOM, we subtract the ensemble mean and divide by the spread. If a control member is specific, it is excluded from the computation of the spread... because that's how it's handled more generally in gen_ens_prod and ensemble_stat.
  • Test this feature branch manually using your own test data in:
    seneca:/d1/projects/MET/MET_pull_requests/met-10.1.0/met-10.1.0-beta6/feature_1918/MET-feature_1918_std_climo_into_develop/met/bin

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

    New output file named gen_ens_prod/gen_ens_prod_NORMALIZE.nc is created. That is the only difference flagged in the GitHub actions for this PR:

ERROR: folder /data/output/met_test_truth missing 1 files
    gen_ens_prod/gen_ens_prod_NORMALIZE.nc 
  • Please complete this pull request review by [Tues 2/22/2022].

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.

@JohnHalleyGotway
Copy link
Collaborator Author

@j-opatz do you have updated requirements to share on this PR? Based on our discussion from this morning, should I remove the FCST_ANOM and FCST_STD_ANOM options from Gen-Ens-Prod? Honestly, that'd be more efficient anyway. Using them requires the tool to read the same data twice... once to compute the mean/spread and once to use that computed mean/spread to normalize each member.

Copy link
Contributor

@j-opatz j-opatz left a comment

Choose a reason for hiding this comment

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

Documentation for the changes looks good, and updates to code still solve the problem in the original issue, with a slight change in approach (now includes series-analysis). Automated testing shows no issues, and code updates appear sound, both reading and during usage.

@JohnHalleyGotway JohnHalleyGotway merged commit ab26b48 into develop Feb 23, 2022
@JohnHalleyGotway JohnHalleyGotway deleted the feature_1918_std_climo branch February 23, 2022 22:53
JohnHalleyGotway added a commit that referenced this pull request Feb 24, 2022
Co-authored-by: Julie Prestopnik <jpresto@seneca.rap.ucar.edu>
Co-authored-by: johnhg <johnhg@ucar.edu>
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>
Co-authored-by: Howard Soh <hsoh@kiowa.rap.ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@kiowa.rap.ucar.edu>
Co-authored-by: jprestop <jpresto@ucar.edu>
Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>
Co-authored-by: Seth Linden <linden@ucar.edu>
Co-authored-by: hsoh-u <hsoh@ucar.edu>
Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com>
Co-authored-by: John Halley Gotway <johnhg@seneca.rap.ucar.edu>
Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>
Co-authored-by: mo-mglover <78152252+mo-mglover@users.noreply.github.com>
Co-authored-by: davidalbo <dave@ucar.edu>
Co-authored-by: lisagoodrich <33230218+lisagoodrich@users.noreply.github.com>
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.

Enhance Gen-Ens-Prod to standardize ensemble members relative to climatology.
2 participants