Bugfix 2638 nccf coordinate dimension main v11.1 #2653

hsoh-u · 2023-08-17T16:57:18Z

Expected Differences

The variable with the standard name "height" and "air_pressure" is selected as the vertical level variable for NCCF. The sample NetCDF from the user has the "height" variable but it's not used as the vertical level. The vertical level will be from the variable's vertical dimension.

        float projection_y_coordinate(projection_y_coordinate) ;
                projection_y_coordinate:axis = "Y" ;
                projection_y_coordinate:standard_name = "projection_y_coordinate" ;
        float projection_x_coordinate(projection_x_coordinate) ;
                projection_x_coordinate:axis = "X" ;
                projection_x_coordinate:standard_name = "projection_x_coordinate" ;
        float percentile(percentile) ;
                percentile:units = "%" ;
                percentile:long_name = "percentile" ;
        float height (height);
                height:units = "m" ;
                height:standard_name = "height" ;
                height:positive = "up" ;
        float visibility_in_air(percentile, projection_y_coordinate, projection_x_coordinate) ;
                visibility_in_air:least_significant_digit = 0LL ;
                visibility_in_air:standard_name = "visibility_in_air" ;
                visibility_in_air:units = "m" ;
                visibility_in_air:grid_mapping = "lambert_azimuthal_equal_area" ;
                visibility_in_air:coordinates = "blend_time forecast_period forecast_reference_time height time" ;

MET recognizes projection_x_coordinate, projection_y_coordinate, and height by the standard_name and the attribute "axis". The "height" is excluded as a coordinate variables because it is not 1 dimension variable. And the variable "visibility_in_air" has the "percentile" dimension as Z. MET checks the vertical level from "percentile", not "height".

Do these changes introduce new tools, command line arguments, or configuration file options? [No]

If yes, please describe:
Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]

If yes, please describe:

Pull Request Testing

Describe testing already performed for these changes:

/d1/personal/hsoh/git/pull_request/MET_bugfix_2638_nccf_coordinate_dimension_main_v11.1/bin/plot_data_plane /d1/personal/hsoh/data/MET-2638/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc plot.ps 'name="visibility_in_air"; level="(1,*,*)";' -v 5

And tested following three:

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

Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
Do these changes include sufficient 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:

An unittest is added. One more output at plot_data_plane//visibility_in_air_by_percentile.ps

Please complete this pull request review by [Fill in date].

Pull Request Checklist

See the METplus Workflow for details.

Review the source issue metadata (required labels, projects, and milestone).
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)
Select: Organization level software support Project or Repository level development cycle Project
Select: Milestone as the version that will include these changes
After submitting the PR, select the ⚙️ icon in the Development section of the right hand sidebar. Search for the issue that this PR will close and select it, if it is not already selected.
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.

…imension variable

jprestop

Hi @hsoh-u. Thank you for making this changes for main v11.1 to resolve the problems as described in #2638. I have reviewed the changes and am waiting to approve to ensure the tests pass. Could you also please add a unit test using the input file?

hsoh-u · 2023-08-17T17:55:35Z

The input file is added to mohawk for unittest at /d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc. unit_test-all.tgz is updated, too

jprestop · 2023-08-17T18:38:09Z

Thank you for your reply @hsoh-u. Based on @JohnHalleyGotway's comment in the issue description:

Recommend adding a unit test using the input file.

I interpreted that as adding a unit test (https://github.com/dtcenter/MET/tree/main_v11.1/internal/test_unit/xml) using the input file. Could you please add a unit test for this in an appropriate xml file?

hsoh-u · 2023-08-17T18:44:41Z

An unit test is added

jprestop · 2023-08-17T19:52:07Z

@hsoh-u I see that one of the tests failed due to:

ERROR :
ERROR : grd_file_type() -> file does not exist "/data/input/MET_test_data/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc"
ERROR :

Is that expected? I know you said that you added it to /d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc and unit_test-all.tgz, so I was surprised to see this fail, but I really don't know the details of the connections. Please let me know if you expected it to fail and I can approve.

hsoh-u · 2023-08-17T21:37:39Z

@hsoh-u I see that one of the tests failed due to:

ERROR :
ERROR : grd_file_type() -> file does not exist "/data/input/MET_test_data/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc"
ERROR :

Is that expected? I know you said that you added it to /d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc and unit_test-all.tgz, so I was surprised to see this fail, but I really don't know the details of the connections. Please let me know if you expected it to fail and I can approve.

Not expected. It looks like the new input data file for unittest is not available at Docker for version 11.1.
There will be procedures to update the unittest input to Docker. I don't know details (for example, if the procedures are automated). I did the first step, updating tar file at mohawk

/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/v11.1/unit_test-all.tgz
/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/develop/unit_test-all.tgz
/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc

hsoh-u · 2023-08-18T15:01:52Z

unit_test-all.tgz at develop and v11.1 were updated yesterday. GHA failed because of newly added input file. What is the next step to add new input file for Docker?

GHA error messages:

DEBUG 1: Opening data file: /data/input/MET_test_data/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc
ERROR  : 
ERROR  : grd_file_type() -> file does not exist "/data/input/MET_test_data/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc"

Status of mohawk for unit test data:

WARNING: YOU ARE met_test@mohawk:/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test$ pwd
/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test
WARNING: YOU ARE met_test@mohawk:/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test$ find -type f -name "unit_test-all.tgz" -ls
861995721 7040304 -rw-r--r--   1 met_test met-dev  7209255177 Aug 17 20:21 ./v11.1/unit_test-all.tgz
862100595 4496852 -rw-r--r--   1 met_test met-dev  4604765515 Feb  5  2023 ./v11.0/unit_test-all.tgz
862037476 7026040 -rw-r--r--   1 met_test met-dev  7194651312 Aug 17 18:02 ./develop/unit_test-all.tgz
WARNING: YOU ARE met_test@mohawk:/d2/www/dtcenter/dfiles/code/METplus/MET/MET_unit_test$ find -type f -name "percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc"
./unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc
./v11.1/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc
./develop/unit_test/model_data/nccf/percentile_extract_20230729T1200Z-B20230725T0030Z-visibility_at_screen_level.nc

jprestop · 2023-08-18T16:09:23Z

@hsoh-u

What is the next step to add new input file for Docker?

I am not sure. I tried poking around in the GitHub Actions files in .github, but I could not find this information. @JohnHalleyGotway are you able to help us out with this information?

JohnHalleyGotway · 2023-08-18T18:31:24Z

FYI, I added the project and milestone to this PR. Please remember to check the Projects, Milestone, and Development metadata when submitting and reviewing to make sure this PR and linked issue get added to the release notes for the target release.

Copy/pasting info over from Slack:
I see the reason for the GHA failure for this PR, and it'll require a small fix to the GHA logic. It's basically version-itis.

The Update input data volumes step is updating the met-data-dev:develop Docker image tag.

data repo: met-data-dev
data version: develop

While the actual unit tests are pulling the met-data-dev:11.1-all Docker image tag.

Status: Downloaded newer image for dtcenter/met-data-dev:11.1-all

But that was never regenerated because the previous step didn't update it!

I will work on the necessary GHA updates on this feature branch now.

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

jprestop

It looks like the only difference is the new file:

dir1: /data/output/met_test_truth contains 1104 files
dir2: /data/output/met_test_output contains 1105 files

ERROR: folder /data/output/met_test_truth missing 1 files
    plot_data_plane/visibility_in_air_by_percentile.ps

I approve.

…mitted to the repo sometime during development.

Co-authored-by: John Halley Gotway <johnhg@ucar.edu> Co-authored-by: Seth Linden <linden@seneca.rap.ucar.edu> Co-authored-by: jprestop <jpresto@ucar.edu> Co-authored-by: Daniel Adriaansen <dadriaan@ucar.edu> Co-authored-by: John and Cindy <halleygotway@Halleys-Mac-mini.local> Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu> Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com> Co-authored-by: hsoh-u <hsoh@ucar.edu> Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu> Co-authored-by: Seth Linden <linden@ucar.edu> Co-authored-by: lisagoodrich <33230218+lisagoodrich@users.noreply.github.com> Co-authored-by: davidalbo <dave@ucar.edu> Co-authored-by: Lisa Goodrich <lisag@ucar.edu> Co-authored-by: metplus-bot <97135045+metplus-bot@users.noreply.github.com> Co-authored-by: j-opatz <59586397+j-opatz@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Jonathan Vigh <jvigh@ucar.edu> Co-authored-by: David Albo <dave@ucar.edu> Co-authored-by: Tracy Hertneky <39317287+hertneky@users.noreply.github.com> Co-authored-by: Dan Adriaansen <dadriaan@ucar.edu> Fix Python environment issue (#2407) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2406) fix #2380 develop override (#2382) fix #2408 develop empty config (#2410) fix #2390 develop compile zlib (#2404) fix #2412 develop climo (#2422) fix #2437 develop convert (#2439) fix for develop, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix #2452 develop airnow (#2454) fix #2449 develop pdf (#2464) fix #2402 develop sonarqube (#2468) fix #2426 develop buoy (#2475) fix 2518 dtypes appf docs (#2519) fix 2531 compilation errors (#2533) fix #2531 compilation_errors_configure (#2535) fix 2596 main v11.1 rpath compilation (#2614) fix #2514 main_v11.1 clang (#2628) fix #2644 main_v11.1 percentile (#2646)

Howard Soh added 2 commits August 10, 2023 07:44

#2638 Execlude air_pressure or height without dimension as vertical d…

572af4f

…imension variable

2638 Override vertical level from the dimension name

8f7b559

hsoh-u requested a review from jprestop August 17, 2023 16:57

hsoh-u linked an issue Aug 17, 2023 that may be closed by this pull request

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

Closed

20 tasks

jprestop requested changes Aug 17, 2023

View reviewed changes

#2638 Added an unittest

45a3c63

hsoh-u requested a review from jprestop August 17, 2023 18:44

JohnHalleyGotway added this to the MET 11.1.1 (bugfix) milestone Aug 18, 2023

jprestop approved these changes Aug 18, 2023

View reviewed changes

Per #2638, remove .find_grid_by_name.h.swp swap file accidentally com…

521e522

…mitted to the repo sometime during development.

hsoh-u merged commit 45d302e into main_v11.1 Aug 18, 2023
32 of 33 checks passed

hsoh-u deleted the bugfix_2638_nccf_coordinate_dimension_main_v11.1 branch August 18, 2023 22:28

JohnHalleyGotway mentioned this pull request Aug 21, 2023

Update main_v11.1-ref after #2653 #2656

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix 2638 nccf coordinate dimension main v11.1 #2653

Bugfix 2638 nccf coordinate dimension main v11.1 #2653

hsoh-u commented Aug 17, 2023 •

edited

Loading

jprestop left a comment

hsoh-u commented Aug 17, 2023

jprestop commented Aug 17, 2023

hsoh-u commented Aug 17, 2023

jprestop commented Aug 17, 2023

hsoh-u commented Aug 17, 2023 •

edited

Loading

hsoh-u commented Aug 18, 2023 •

edited

Loading

jprestop commented Aug 18, 2023

JohnHalleyGotway commented Aug 18, 2023 •

edited

Loading

jprestop left a comment

Bugfix 2638 nccf coordinate dimension main v11.1 #2653

Bugfix 2638 nccf coordinate dimension main v11.1 #2653

Conversation

hsoh-u commented Aug 17, 2023 • edited Loading

Expected Differences

Pull Request Testing

Pull Request Checklist

jprestop left a comment

Choose a reason for hiding this comment

hsoh-u commented Aug 17, 2023

jprestop commented Aug 17, 2023

hsoh-u commented Aug 17, 2023

jprestop commented Aug 17, 2023

hsoh-u commented Aug 17, 2023 • edited Loading

hsoh-u commented Aug 18, 2023 • edited Loading

jprestop commented Aug 18, 2023

JohnHalleyGotway commented Aug 18, 2023 • edited Loading

jprestop left a comment

Choose a reason for hiding this comment

hsoh-u commented Aug 17, 2023 •

edited

Loading

hsoh-u commented Aug 17, 2023 •

edited

Loading

hsoh-u commented Aug 18, 2023 •

edited

Loading

JohnHalleyGotway commented Aug 18, 2023 •

edited

Loading