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 1657 bounds attribute #1674

Merged
merged 5 commits into from
Feb 22, 2021
Merged

Feature 1657 bounds attribute #1674

merged 5 commits into from
Feb 22, 2021

Conversation

hsoh-u
Copy link
Collaborator

@hsoh-u hsoh-u commented Feb 18, 2021

Pull Request Testing

  • Describe testing already performed for these changes:

CF allows to set bounds attribute. MET override the time data (for accumulation) from the associated bounds variable (by "bounds" attribute) instead of the time variable.
Found the 1 second off (valid time: 20190601_145959 should be valid time: 20190601_150000) by the precision error during testing. It's fixed

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

The test input file is located at dakota:/d1/projects/MET/MET_test_data/unit_test/model_data/nccf/gpm_2019060103_2019060203.nc

The command to test:

regrid_data_plane /d1/projects/MET/MET_test_data/unit_test/model_data/nccf/gpm_2019060103_2019060203.nc G211 gpm_2019060103_2019060203_precipitation.nc -field 'name="precipitation_amount"; level="(*,*)";' -v 4
  • Do these changes include sufficient documentation and testing updates? [No]

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

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

The bug was fixed, so time of hh:59:59 or hh:00:01 will be fixed to hh:00:00 or (hh+1):00:01. I'm not sure if unit tests have this case.

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.

Command to test:

regrid_data_plane /d1/projects/MET/MET_test_data/unit_test/model_data/nccf/gpm_2019060103_2019060203.nc G211 gpm_2019060103_2019060203_precipitation.nc -field 'name="precipitation_amount"; level="(*,*)";' -v 4

Before:

DEBUG 4: Data plane information:
DEBUG 4:       plane min: 0
DEBUG 4:       plane max: 247.173
DEBUG 4:      valid time: 20190601_145959
DEBUG 4:       lead time: 000000
DEBUG 4:       init time: 20190601_145959
DEBUG 4:      accum time: 000000

        float precipitation_amount(lat, lon) ;
                precipitation_amount:init_time = "20190601_145959" ;
                precipitation_amount:init_time_ut = "1559401199" ;
                precipitation_amount:valid_time = "20190601_145959" ;
                precipitation_amount:valid_time_ut = "1559401199" ;

After (valid time, init time, & accum time are changed)

DEBUG 3: NcCfFile::open() -> read time from the bounds variable "time_bnds"
...
DEBUG 4: Data plane information:
DEBUG 4:       plane min: 0
DEBUG 4:       plane max: 247.173
DEBUG 4:      valid time: 20190602_030000
DEBUG 4:       lead time: 000000
DEBUG 4:       init time: 20190602_030000
DEBUG 4:      accum time: 240000

        float precipitation_amount(lat, lon) ;
                precipitation_amount:init_time = "20190602_030000" ;
                precipitation_amount:init_time_ut = "1559444400" ;
                precipitation_amount:valid_time = "20190602_030000" ;
                precipitation_amount:valid_time_ut = "1559444400" ;
                precipitation_amount:accum_time = "240000" ;
                precipitation_amount:accum_time_sec = 86400 ;

@hsoh-u hsoh-u added this to the MET 10.0.0 milestone Feb 18, 2021
@hsoh-u hsoh-u added this to In progress in MET-10.0.0-beta4 (3/2/21) via automation Feb 18, 2021
@hsoh-u hsoh-u linked an issue Feb 18, 2021 that may be closed by this pull request
21 tasks
@hsoh-u hsoh-u moved this from In progress to Pull request review in MET-10.0.0-beta4 (3/2/21) Feb 18, 2021
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway left a comment

Choose a reason for hiding this comment

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

Thanks Howard! I tested the regrid_data_plane command you recommended and confirmed that it worked. I did try digging into the details of add_to_unixtime() trying to simplify it a bit, but couldn't find a simpler solution. Those precision issues with large numbers can get very tricky! I approve of these changes.

@hsoh-u hsoh-u merged commit 5e3d6d1 into develop Feb 22, 2021
MET-10.0.0-beta4 (3/2/21) automation moved this from Pull request review to Done Feb 22, 2021
@hsoh-u hsoh-u deleted the feature_1657_bounds_attribute branch February 22, 2021 18:33
georgemccabe added a commit to dtcenter/METplus that referenced this pull request Feb 23, 2021
Co-authored-by: bikegeek <minnawin@ucar.edu>
Co-authored-by: bikegeek <3753118+bikegeek@users.noreply.github.com>
Co-authored-by: Julie.Prestopnik <jpresto@ucar.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Support "bounds" attribute as CF for time bounds
2 participants