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.

Sign up for GitHub

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

Jump to bottom

Support "bounds" attribute as CF for time bounds #1657

Closed
7 of 21 tasks
hsoh-u opened this issue Feb 9, 2021 · 1 comment · Fixed by #1674 or #1680
Closed
7 of 21 tasks

Support "bounds" attribute as CF for time bounds #1657

hsoh-u opened this issue Feb 9, 2021 · 1 comment · Fixed by #1674 or #1680
Assignees
@hsoh-u
Labels
MET: Library Code requestor: UK Met Office United Kingdom Met Office type: new feature Make it do something new
Milestone
MET 10.0.0

Comments

@hsoh-u
Copy link
Collaborator

hsoh-u commented Feb 9, 2021 •
edited
Loading

CF complaint describes "bounds"attribute. MET should support the bound attribute and save the time value from the "bounds" variable instead of the time variable.
From RT (https://rt.rap.ucar.edu/rt/Ticket/Display.html?id=98391)

Describe the New Feature

Adding the following logic to MET's NetCDF CF library:

  • When parsing time variables, check for the "bounds" attribute.
  • If present, it should specify the name of the variable containing the time bounds.
  • Parse the times from that variable.
  • Define valid_time = max of those times
  • Define accum_time = max - min of those times

Acceptance Testing

I need a sample file with "bounds" attribute.

Time Estimate

1 or 2 days

Sub-Issues

Consider breaking the new feature down into sub-issues.

  • Add a checkbox for each sub-issue here.

Relevant Deadlines

Need Rachel to confirm that logic is correct.

Funding Source

Define the source of funding and account keys here or state NONE.

Define the Metadata

Assignee

  • Select engineer(s) or no engineer required (Hsoh)
  • Select scientist(s) or no scientist required (None)

Labels

  • Select component(s)
  • Select priority
  • Select requestor(s)

Projects and Milestone

  • Review projects and select relevant Repository and Organization ones or add "alert:NEED PROJECT ASSIGNMENT" label
  • Select milestone to next major version milestone or "Future Versions"

Define Related Issue(s)

Consider the impact to the other METplus components.

New Feature Checklist

See the METplus Workflow for details.

  • Complete the issue definition above, including the Time Estimate and Funding source.
  • Fork this repository or create a branch of develop.
    Branch name: feature_<Issue Number>_<Description>
  • Complete the development and test your changes.
  • Add/update log messages for easier debugging.
  • Add/update unit tests.
  • Add/update documentation.
  • Push local changes to GitHub.
  • Submit a pull request to merge into develop.
    Pull request: feature <Issue Number> <Description>
  • Define the pull request metadata, as permissions allow.
    Select: Reviewer(s), Project(s), Milestone, and Linked issues
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@hsoh-u hsoh-u added component: library code type: new feature Make it do something new requestor: UK Met Office United Kingdom Met Office alert: NEED ACCOUNT KEY Need to assign an account key to this issue labels Feb 9, 2021
@hsoh-u hsoh-u added this to the MET 10.0.0 milestone Feb 9, 2021
@hsoh-u hsoh-u self-assigned this Feb 9, 2021
@JohnHalleyGotway
Copy link
Collaborator

Here's a sample data file. I added the .txt suffix so that GitHub would let me attach it!
gpm_2019060103_2019060203.nc.txt

hsoh-u pushed a commit that referenced this issue Feb 18, 2021
#1657 Added TIME_EPSILON
25e7d4f
hsoh-u pushed a commit that referenced this issue Feb 18, 2021
#1657 Corrected 1 second offset by the precision error
9b8ccf2
hsoh-u pushed a commit that referenced this issue Feb 18, 2021
#1657 Added AccumTime
b72f0b6
hsoh-u pushed a commit that referenced this issue Feb 18, 2021
#1657 Read the time from "bounds" attribute and set the max value fro…
596562a 
…m the bounds time variable
hsoh-u pushed a commit that referenced this issue Feb 18, 2021
#1657 Corrected 1 second offset by the precision error
3668fa4
@hsoh-u hsoh-u linked a pull request Feb 18, 2021 that will close this issue
Feature 1657 bounds attribute #1674
Merged
10 tasks
@hsoh-u hsoh-u mentioned this issue Feb 23, 2021
Closed
21 tasks
@hsoh-u hsoh-u linked a pull request Feb 23, 2021 that will close this issue
Bugfix 1677 update_reference_time #1680
Merged
10 tasks
JohnHalleyGotway added a commit that referenced this issue Feb 25, 2021
@JohnHalleyGotway @hsoh-u
@jprestop @j-opatz
Update develop-ref after #1681 and #1676 (#1683)
ba0cc8e 
* Per 1646, one line fix for cut-and-paste error. (#1647)

* Per #1644, no actual code changes here. Just formatting and spacing. For example, replace double ;; with single ;'

* Per #1644, FOUND THE BUG! It's a copy/paste error. We had var_name_map.end() that should be def_var_name_map.end(). Fixing that gets rid of the runtime hang.'

* Per #1643, redefine the contents of the existing AREA_RATIO output column 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)

* Feature 1644 ps_log (#1651)

* 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.

* The mode_conv.pl logic was slightly broken. MET PR #1650 should have 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.

* The diffing logic for MODE pair lines still was not correct. We'd added 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!

* Feature 1653 rscripts (#1654)

* 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.

* Feature 1655 nc_log (#1656)

* #1630 Display a warning instead of error message with invalid variable if the input data is empty

* Feature 1658 grib_tables (#1659)

* Per #1658, update MXUPHL entries.

* Per #1658, updating long name for MAXREF, MAXUVV, and MAXDVV.

* Modified format of release notes

* Feature 1450 hersbach (#1662)

* 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.

* #1657 Added TIME_EPSILON

* #1657 Corrected 1 second offset by the precision error

* #1657 Added AccumTime

* #1657 Read the time from "bounds" attribute and set the max value from the bounds time variable

* #1657 Corrected 1 second offset by the precision error

* Per #1439, add check_mask_names() utility function which errors out if the list of masking region names is non-unique. Update Point-Stat and Grid-Stat to call it. (#1679)

* Feature 1451 crpss (#1676)

* 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 <[email protected]>

* #1677 Update the refence time (from time_bnds variable) (#1680)

Co-authored-by: Howard Soh <[email protected]>

* Feature 1135 stat_analysis (#1681)

* 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 <[email protected]>

Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: jprestop <[email protected]>

Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: Julie.Prestopnik <[email protected]>
Co-authored-by: j-opatz <[email protected]>
@hsoh-u hsoh-u closed this as completed Mar 1, 2021
@TaraJensen TaraJensen removed the alert: NEED ACCOUNT KEY Need to assign an account key to this issue label Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hsoh-u hsoh-u

Labels
MET: Library Code requestor: UK Met Office United Kingdom Met Office type: new feature Make it do something new
Projects
None yet
Milestone
MET 10.0.0
Development

Successfully merging a pull request may close this issue.

Feature 1657 bounds attribute dtcenter/MET
Bugfix 1677 update_reference_time dtcenter/MET
3 participants
@hsoh-u @JohnHalleyGotway @TaraJensen