-
Notifications
You must be signed in to change notification settings - Fork 24
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 #1681 and #1676 #1683
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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)
…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.
…e if the input data is empty
Feature 1630 point2grid hang
* 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.
…m the bounds time variable
Feature 1657 bounds attribute
…f the list of masking region names is non-unique. Update Point-Stat and Grid-Stat to call it. (#1679)
* 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. * Per #1451, instead of computing the climo crps on the fly, compute and store it separately for each point. * Per #1451, the ECNT line type will no longer be written separately for each CDF bin. Removing the bin-related arguments from the write_ecnt functions. * Per #1451, the climo_cdf.write_bins option no longer applies. Since Ensemble-Stat will no longer compute stats separately for each climo bin, I'm removing the reference to write_bins from the Ensemble-Stat config files. * Per #1451, compute and store the climo CRPS for each point. Also, break apart the normal climo computation into separate functions for crps, ign, and pit. * Per #1451, update Ensemble-Stat logic to no longer subset pairs into climo CDF bins. We had done this to be consistent with the use of climo data in point and grid-stat. But this change to the handling of climo data is consitent with the NOAA/EMC approach. * Per #1451, split out the setting of climo CDF thresholds into a separate function so that it can also be called by stat-analysis. * Per #1451, in the Ensemble-Stat ORANK line type, rename CLIMO to CLIMO_MEAN and add a CLIMO_STDEV column. * Per #1451, also need to update gsidens2orank to write a climo_stdev column. * Per #1451, switch from constant pointer to ClimoCDFInfo object to a copy to make the logic of doing this in Stat-Analysis a little easier. * Per #1451, the HiRA method in Point-Stat computes an ECNT output line type. Needed to call set_climo_cdf() there so that we know how many climo values to use when computing the empirical climo CRPS. * Per #1451, need to store climo_cdf for both grid and point verification. * Per #1451, update to write the CLIMO_STDEV header column for the ORANK line type. * Per #1451, in Ensemble-Stat when doing point verification, check for empty OBS_UNIT string and write NA instead. * Per #1451, update unit tests by enhancing the climatology call to Ensemble-Stat to also include point verification. Tweak the Ensemble-Stat cofiguration for that and also add a call to pb2nc to prepare the point observations for use. * Per #1450, added a new section to the Ensemble-Stat chapter describing how climo mean/stdev are used in the computation of the skill scores. * Update ensemble-stat.rst Co-authored-by: j-opatz <59586397+j-opatz@users.noreply.github.com>
Co-authored-by: Howard Soh <hsoh@kiowa.rap.ucar.edu>
* Per #1135, add fcst/obs_init/valid_inc/exc options for STAT-Analysis jobs. * Per #1135, update all the STATAnalysis config files to include entries for the new fcst/obs_init/valid_inc/exc options. * Per #1135, add documentation for fcst/obs_init/valid_inc/exc options to the STAT-Analysis chapter. Also, clarify the description for the existing options. * Per #1135, adding another call to stat_analysis to check the time filtering options. * Per #1135, just renaming stat_analysis output file. * Apply suggestions from code review Co-authored-by: jprestop <jpresto@ucar.edu> Co-authored-by: John Halley Gotway <johnhg@kiowa.rap.ucar.edu> Co-authored-by: jprestop <jpresto@ucar.edu>
22 tasks
jprestop
approved these changes
Feb 25, 2021
There was a problem hiding this 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. Thanks for working through the merging process!
JohnHalleyGotway
added a commit
that referenced
this pull request
Feb 25, 2021
…dvertently dropped when manually merging changes for PR #1683.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Testing
For some reason, it looks like the PR won't automatically merge. Once you approve, I'll work through the merging process.
The MET NB for the develop branch failed on kiowa for NB20210225. The differences are all expected and caused by 2 recent pull requests.
MET PR #1681 for issue #1135 adds a new Stat-Analysis output file for testing the new job command options:
MET PR #1676 for issue #1451 makes the following changes:
I looked through the diffs in these 32 files and confirmed that they're all based on:
For ORANK, renaming CLIMO to CLIMO_MEAN and adding CLIMO_STDEV to the end.
For ECNT, change the definition of the CLIMOCL computation and therefore the CRPSS score.
For ECNT, remove the computation of ECNT stats for each climo bin and then report the mean of the bins.
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 and testing updates? [Yes]
This PR serves as documentation for why we updated the reference.
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.
Select: Reviewer(s), Project(s), and Milestone