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

Bugfix: Refine support for coordinate dimensions in CF-compliant NetCDF files #2638

Closed
7 of 20 tasks
JohnHalleyGotway opened this issue Aug 3, 2023 · 2 comments · Fixed by #2653
Closed
7 of 20 tasks
Assignees
Labels
MET: Plotting Tools priority: high High Priority requestor: UK Met Office United Kingdom Met Office required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: bug Fix something that is not working

Comments

@JohnHalleyGotway
Copy link
Collaborator

JohnHalleyGotway commented Aug 3, 2023

Describe the Enhancement

This issue arose via GitHub discussion dtcenter/METplus#2273 from @robdarvell and is demonstrated by running the plot_data_plane tool. Use this file to demonstrate the issue:
percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc.gz

The goal is to retrieve 2D fields from this variable:
float visibility_in_air(percentile, projection_y_coordinate, projection_x_coordinate) ;

Plotting the 0-th percentile index works as expected:
plot_data_plane percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc plot.ps 'name="visibility_in_air"; level="(0,*,*)";'
Screen Shot 2023-08-03 at 12 39 42 PM
Plotting the VALUE of the first percentile also works fine:
plot_data_plane percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc plot.ps 'name="visibility_in_air"; level="(@5,*,*)";'

However, plotting ANY OTHER index or percentile value produces a runtime error:

  • name="visibility_in_air"; level="(1,*,*)";'
  • name="visibility_in_air"; level="(@10,*,*)";'
  • name="visibility_in_air"; level="(6,*,*)";'
  • name="visibility_in_air"; level="(@50,*,*)";'

Here's the error:

ERROR  : 
ERROR  : MetNcCFDataFile::data_plane(VarInfo &, DataPlane &) -> the requested vertical value 50 for "visibility_in_air" variable does not exist (data size = 1).
ERROR  : 

I assumed that the changes for #1815 included in MET version 11.0.0 would enable this logic to work. In general, for an N-dimensional NetCDF variable, exactly 2 of those dimensions should define the X/Y dimensions. The remaining dimensions should be settable by specifying their index as an integer. Additionally, for coordinate variables (i.e. dimension and 1-d variable of the same name), MET should support the @ syntax to specify the value for that dimension.

This issue is to refine the logic to support the processing of this data.

Please fix this in the main_v11.1 branch to be included in an 11.1.1 bugfix release. Please also fix this in the develop branch to be included in the 12.0.0 official release.

Recommend adding a unit test using the input file.

Time Estimate

2 days?

Sub-Issues

Consider breaking the enhancement down into sub-issues.
None needed.

Relevant Deadlines

List relevant project deadlines here or state NONE.

Funding Source

2799991

Define the Metadata

Assignee

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

Labels

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

Projects and Milestone

  • Select Repository and/or Organization level Project(s) or add alert: NEED CYCLE ASSIGNMENT label
  • Select Milestone as the next official version or Future Versions

Define Related Issue(s)

Consider the impact to the other METplus components.

Enhancement 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) and Development issues
    Select: Repository level development cycle Project for the next official release
    Select: Milestone as the next official version
  • Iterate until the reviewer(s) accept and merge your changes.
  • Delete your fork or branch.
  • Close this issue.
@JohnHalleyGotway JohnHalleyGotway added type: enhancement Improve something that it is currently doing requestor: UK Met Office United Kingdom Met Office required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project MET: Plotting Tools priority: high High Priority labels Aug 3, 2023
@JohnHalleyGotway JohnHalleyGotway added this to the MET 12.0.0 milestone Aug 3, 2023
@JohnHalleyGotway JohnHalleyGotway added type: bug Fix something that is not working and removed type: enhancement Improve something that it is currently doing labels Aug 8, 2023
@JohnHalleyGotway
Copy link
Collaborator Author

@hsoh-u, FYI, during the METplus engineering meeting on Tuesday 8/8, the team decided to reclassify this issue as a bug instead of an enhancement. As such, please fix this in the main_v11.1 branch first.

@hsoh-u
Copy link
Collaborator

hsoh-u commented Aug 8, 2023

MET picks the "height" variable as the vertical level instead of "percentile" because of the standard name. The "height" variable has only 1 value. This caused problem, only offset 0 works for vertical offset.

MET code sets the coordinate variables by the standard names. The standard names as the coordinate variables are time, latitude, longitude, air_pressure, and height. The solution will be exclude them by looking at the dimensions.

The workaround is removing the standard name and adding the long_name at the "height" variable.

        float percentile(percentile) ;
                percentile:units = "%" ;
                percentile:long_name = "percentile" ;
        float height ;
                height:units = "m" ;
                height:standard_name = "height" ;
                height:positive = "up" ;

hsoh-u pushed a commit that referenced this issue Aug 10, 2023
@hsoh-u hsoh-u moved this from Todo to In Progress in Coordinated METplus-5.1 Support Aug 10, 2023
@JohnHalleyGotway JohnHalleyGotway changed the title Refine support for coordinate dimensions in CF-compliant NetCDF files Bugfix: Refine support for coordinate dimensions in CF-compliant NetCDF files Aug 16, 2023
@hsoh-u hsoh-u linked a pull request Aug 17, 2023 that will close this issue
15 tasks
hsoh-u pushed a commit that referenced this issue Aug 17, 2023
JohnHalleyGotway added a commit that referenced this issue Aug 18, 2023
…itch from v2 to the most recent v3. Also, instead of setting branch_name to the current branch being run, switch to using truth_data_version. That will be set to develop or main_vX.Y. For this branch, its set to main_v11.1 which will tell the METplus action to update the corresponding input data. Not 100% sure this will work, but its worth a try.
JohnHalleyGotway added a commit that referenced this issue Aug 18, 2023
…mitted to the repo sometime during development.
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Coordinated METplus-5.1 Support Aug 18, 2023
@JohnHalleyGotway JohnHalleyGotway moved this from 🔖 Ready to ✅ Done in MET-12.0.0 Development Aug 23, 2023
@hsoh-u hsoh-u mentioned this issue Dec 13, 2023
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MET: Plotting Tools priority: high High Priority requestor: UK Met Office United Kingdom Met Office required: FOR DEVELOPMENT RELEASE Required to be completed in the development release for the assigned project type: bug Fix something that is not working
Projects
Status: 🏁 Done
2 participants