-
Notifications
You must be signed in to change notification settings - Fork 26
Update develop-ref after #1662 #1669
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
Merged
Merged
Conversation
This file contains hidden or 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.
18 tasks
j-opatz
approved these changes
Feb 17, 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.
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
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.
After merging into develop, I reran MET's regressions tests on kiowa. It flagged differences in 25 files.
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.
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:
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.
Select: Reviewer(s), Project(s), and Milestone