Skip to content

Conversation

JohnHalleyGotway
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway commented Feb 17, 2021

Pull Request Testing

PR #1662 for issue #1450 added 4 new columns to the ECNT line type. This PR updates develop-ref to get past the resulting differences in the output.

  • Describe testing already performed for these changes:

    After merging into develop, I reran MET's regressions tests on kiowa. It flagged differences in 25 files.
cd /d1/projects/MET/MET_regression/develop/NB20210217
egrep -i "file1:|ERROR:" test_regression_20210217.log  | egrep -B 1 "ERROR:" | grep file1
file1: MET-develop-ref/test_output/climatology/ensemble_stat_NCEP_1.0DEG_20120410_120000V.stat
file1: MET-develop-ref/test_output/climatology/point_stat_WMO_CLIMO_1.5DEG_120000L_20120409_120000V_ecnt.txt
file1: MET-develop-ref/test_output/climatology/point_stat_WMO_CLIMO_1.5DEG_120000L_20120409_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_CMD_LINE_20120410_120000V_ecnt.txt
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_CMD_LINE_20120410_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_FILE_LIST_20120410_120000V_ecnt.txt
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_FILE_LIST_20120410_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_MASK_SID_20120410_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_MASK_SID_CENSOR_20120410_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_OBS_ERROR_20120410_120000V_ecnt.txt
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_OBS_ERROR_20120410_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_QTY_MADIS_VGS_20120409_120000V_ecnt.txt
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_QTY_MADIS_VGS_20120409_120000V.stat
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_SKIP_CONST_20120410_120000V_ecnt.txt
file1: MET-develop-ref/test_output/ensemble_stat/ensemble_stat_SKIP_CONST_20120410_120000V.stat
file1: MET-develop-ref/test_output/grid_weight/ensemble_stat_AREA_WEIGHT_20120410_120000V.stat
file1: MET-develop-ref/test_output/grid_weight/ensemble_stat_COS_LAT_WEIGHT_20120410_120000V.stat
file1: MET-develop-ref/test_output/grid_weight/ensemble_stat_NO_WEIGHT_20120410_120000V.stat
file1: MET-develop-ref/test_output/hira/point_stat_HIRA_EMPTY_PROB_CAT_THRESH_360000L_20120410_120000V.stat
file1: MET-develop-ref/test_output/hira/point_stat_NCMET_NAM_HMTGAGE_HIRA_360000L_20120410_120000V.stat
file1: MET-develop-ref/test_output/met_test_scripts/ensemble_stat/ensemble_stat_20100101_120000V_ecnt.txt
file1: MET-develop-ref/test_output/met_test_scripts/ensemble_stat/ensemble_stat_20100101_120000V.stat
file1: MET-develop-ref/test_output/python/ensemble_stat_PYTHON_20050807_120000V.stat
file1: MET-develop-ref/test_output/stat_analysis/AGG_STAT_ORANK_ECNT.out
file1: MET-develop-ref/test_output/stat_analysis/AGG_STAT_ORANK_ECNT_out.stat

Note that the differences are confined to ECNT output type. I ran "tkdiff" to spot check a few of the files and confirm that the diffs are in the expected columns.

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

    Assigning to @j-opatz since he reviewed Feature 1450 hersbach #1662. John, if you're not available, please reassign to @jprestop since she's done these before. Feel free to spot check as much output as you'd like:
cd /d1/projects/MET/MET_regression/develop/NB20210217
/usr/bin/tkdiff MET*/test_output/ensemble_stat/ensemble_stat_CMD_LINE_20120410_120000V_ecnt.txt
  • Do these changes include sufficient documentation and testing updates? [Yes]
    This PR documents the reason for updating develop-ref.

  • Will this PR result in changes to the test suite? [Yes]

    If yes, describe the new output and/or changes to the existing output:

    Yes, as described above.

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.

JohnHalleyGotway and others added 15 commits January 28, 2021 16:31
…For example, replace double ;; with single ;'
…p.end() that should be def_var_name_map.end(). Fixing that gets rid of the runtime hang.'
…lumn from MODE. Define it as FCST/OBS object area instead of min/max. Update the User's Guide to note the change and also clarify that the MTD VOLUME_RATIO output really is FCST/OBS. (#1650)
* Per #1644, write rejection reason codes at verbosity 2 when there are 0 matched pairs.

* Per #1644, add a few sentences to Point-Stat, Practical Information chapter about debugging 0 matched pairs.
…broken the NB but it did not. Turns out the diffing logic is NOT properly distinguishing between single and pair object lines. It does this by looking for an underscore in the OBJECT_ID column. When we added FCST_UNITS and OBS_UNITS, that shifted OBJECT_ID up 2 spots, but the code was still checking the (0-based) 20th column instead of the 22nd. Fixing this now and will rerun NB20210202 to confirm it works again.
…ed the ASPECT_DIFF and CURVATURE_RATIO columns a while ago, but they were missing from the diff logic. This logic really is not good. We need to make it more robust, reading the version-specific header columns from a table file instead of hard-coding them!
* Per #1653, update plot_cnt.R and plot_mpr.R to remove the version-specific header columns.

* Per #1653, nice enhancments to these Rscripts to make them more independent of the MET version number.

* Per #1653, more tweaks

* Per #1653, if no input files are provided, error out with a useful message.

* Per #1653, while the scripts ran fine using R 4.0.2 on my Mac, they fail on eyewall using R 3.4.0. Adding as.character() to get past that error.
* Per #1658, update MXUPHL entries.

* Per #1658, updating long name for MAXREF, MAXUVV, and MAXDVV.
* Per #1450, add new ECNT columns for Hersback CRPS. Still need to actually compute the stats though.

* Per #1450, update NumArray functions to only sort if the data is not yet sorted. And check for bad data when computing the standard deviation.

* Per #1450, add code to compute the empirical CRPS value.

* Per #1450, large change to the new output for the empirical CRPS. In order to aggregate decomposed empirical CRPS reliability and potential correctly, we'd need to write (n+1)*2 additional columns. While the empirical crps can be aggregated as a weighted mean, the decomposition cannot. It just isn't feasible to do this in the ECNT line type. If this reliability and potential really are required, recommend that we add an entirely new CRPS line type instead of tacking onto ECNT. These changes simply remove reliabilit and potential from the output.

* Per #1450 and #1451, replacing single CRPS_CLIMO column with CRPSCL and CRPSCL_EMP which will be needed for #1451.

* Per #1450, delete temp files I'd accidentally committed.

* Per #1450, update the user's guide with CRPS updates.

* Fix bug replacing crpss_emp with crpss_gaus.
@JohnHalleyGotway JohnHalleyGotway added this to the MET 10.0.0 milestone Feb 17, 2021
@JohnHalleyGotway JohnHalleyGotway linked an issue Feb 17, 2021 that may be closed by this pull request
18 tasks
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.

There were several files that had changes unrelated to #1450 so while I can't speak to their origins/needs, they seemed correct. All files affected by #1450 were checked and appear correct for what is needed.

@JohnHalleyGotway JohnHalleyGotway merged commit e9ec2c1 into develop-ref Feb 18, 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.

Add the Hersbach CRPS and CRPSS statistics to the ECNT line type.
4 participants