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 2867 point2grid qc flag main v11.1 #2874

Merged
merged 4 commits into from
May 3, 2024

Conversation

hsoh-u
Copy link
Collaborator

@hsoh-u hsoh-u commented Apr 30, 2024

Expected Differences

The meaning of ADP QC values were changed (it was 3 for high, 2 for medium, and 1 for low).
The baseline algorithm and the enterprise algorithm produce different QC values for high, medium, and low. MET reads QC values and meanings from the variable attribute and apply them to -qc options (where 0 is high, 1 is medium, and 2 is low).

  • QC values for Baseline algorithm:
    • high: 3 (raw value: 12 or 48)
    • medium: 1 (raw value: 4 or 16)
    • low: 0
  • QC values for Enterprise algorithm:
    • high: 0
    • medium: 1 (raw value: 4 or 16)
    • low: 2 (raw value: 8 or 32)

The -qc options at the unittests were changed to -qc 0,1,2.

  • 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:

New GOES16 data with Enterprise algorithm:

/d1/personal/hsoh/git/pull_request/MET_bugfix_2867_point2grid_qc_flag_main_v11.1/bin/point2grid  /d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/OR_ABI-L2-AODC-M6_G16_s20241100001171_e20241100003544_c20241100006242.nc G212 goes_aod_smoke_adp_high2.nc -adp /d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/OR_ABI-L2-ADPC-M6_G16_s20241100001171_e20241100003544_c20241100006361.nc -field 'name="AOD_Smoke"; level="*";' -v 4 -qc 0,1

==>

DEBUG 4: set_adp_gc_values()  high_confidence = 0, medium_confidence = 1, low_confidence = 2

DEBUG 4: regrid_goes_variable() -> Count: actual: 6, missing: 2758918, non_missing: 652344
DEBUG 4:    Filtered: by QC: 0, by adp QC: 62127, by absent: 590193, total: 652320
DEBUG 4:    Range:  data: [-0.05 - 4.99997]  QC: [0 - 2]
DEBUG 4:    AOD QC: high=8092 medium=6910, low=47149, no_retrieval=0
DEBUG 4:    ADP QC: high=4 medium=87, low=62050, no_retrieval=10, adjusted=47142

Note: if only high quality is given with-qc 0, all data will be filtered out.

Meaning of the debug message DEBUG 4: ADP QC: high=4 medium=87, low=62050, no_retrieval=10, adjusted=47142

  • number of high confidence: 4
  • number of medium confidence=87
  • number of low confidence=62050
  • number of no_retrieval_qf=10
  • number of adjusted confidences=47142: confidences were reduced by combining with data QC flags

Old GOES16 data with Baseline algorithm:

/d1/personal/hsoh/git/pull_request/MET_bugfix_2867_point2grid_qc_flag_main_v11.1/bin/point2grid  /d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/OR_ABI-L2-AODC-M6_G16_s20192662141196_e20192662143569_c20192662145547.nc G212 goes_aod_smoke_adp_high2.nc -adp /d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/OR_ABI-L2-ADPC-M6_G16_s20192662141196_e20192662143569_c20192662144526.nc -field 'name="AOD_Smoke"; level="*";' -v 4 -qc 0,1

==>

DEBUG 4: set_adp_gc_values()  high_confidence = 3, medium_confidence = 1, low_confidence = 0

DEBUG 4: regrid_goes_variable() -> Count: actual: 121, missing: 1937596, non_missing: 1473666
DEBUG 4:    Filtered: by QC: 0, by adp QC: 86130, by absent: 1387116, total: 1473246
DEBUG 4:    Range:  data: [-0.05 - 4.99997]  QC: [0 - 2]
DEBUG 4:    AOD QC: high=222 medium=1938, low=84390, no_retrieval=0
DEBUG 4:    ADP QC: high=634 medium=9, low=85907, no_retrieval=0, adjusted=84770
  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:

More AOD files with Enterprise algorithm are at seneca:/d1/personal/hsoh/data/MET-2853/20240419

  • 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? [Yes]
    One unit test was added and -qc options were changed.

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

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

New output for Enterprise algorithm: point2grid/point2grid_GOES_16_ADP_Enterprise_high.nc

Three existing output will be different because -qc 1,2,3 was changed to -qc 0,1,2:

  • point2grid/point2grid_GOES_16_ADP.nc

  • point2grid_GOES_16_AOD_TO_G212_grid_map.nc

  • point2grid_GOES_16_AOD_TO_G212.nc

  • Will this PR result in changes to existing METplus Use Cases? [Yes]

    If yes, create a new Update Truth METplus issue to describe them.

The meaning of QC values were changed. So the output will be different if -qc option is given.

  • Do these changes introduce new SonarQube findings? [Yes or No]

    If yes, please describe:

  • 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) and Development issue
    Select: Milestone as the version that will include these changes
    Select: Coordinated METplus-X.Y Support project for bugfix releases or MET-X.Y.Z Development project for official releases
  • 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.

Howard Soh added 4 commits April 30, 2024 19:31
…t GOES16 Enterprise allgorithm) and apply them for QC Flags. Adjusted the ADP QC flags based on the variable QC values
@jprestop
Copy link
Collaborator

jprestop commented May 2, 2024

@hsoh-u
I see that you pointed me to the Enterprise AOD files, but I am wondering if I should also be testing the Baseline files? If so, could you please point me to those as well?

@jprestop
Copy link
Collaborator

jprestop commented May 2, 2024

@hsoh-u Actually, I think I may have found them from the section where you listed what testing you did.

/d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/

Apologies for not catching that before I sent the last message.

@jprestop
Copy link
Collaborator

jprestop commented May 2, 2024

@hsoh-u Can you please confirm that the failed check for differences is expected?

@hsoh-u
Copy link
Collaborator Author

hsoh-u commented May 2, 2024

@hsoh-u Can you please confirm that the failed check for differences is expected?

One output is added and three files are expected to be different because the logic to identify high, medium, and low confidences is changed.

  • point2grid/point2grid_GOES_16_ADP.nc
  • point2grid_GOES_16_AOD_TO_G212_grid_map.nc
  • point2grid_GOES_16_AOD_TO_G212.nc

@hsoh-u
Copy link
Collaborator Author

hsoh-u commented May 2, 2024

@hsoh-u Actually, I think I may have found them from the section where you listed what testing you did.

/d1/projects/MET/MET_test_data/unit_test/model_data/goes_16/

Apologies for not catching that before I sent the last message.

More data at seneca:/d1/personal/hsoh/data/MET-2853/20240419

@jprestop
Copy link
Collaborator

jprestop commented May 3, 2024

Thanks @hsoh-u Can you please confirm that the failed Check for Differences is expected?

@hsoh-u
Copy link
Collaborator Author

hsoh-u commented May 3, 2024

Thanks @hsoh-u Can you please confirm that the failed Check for Differences is expected?

Yes, three output files from GOES-16 inputs are different because of different logic for high, medium, and low confidences.

Copy link
Collaborator

@jprestop jprestop left a comment

Choose a reason for hiding this comment

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

I ran the modified code on the baseline and enterprise files and saw the same behavior that you did in your testing for this PR. You confirmed that the failure in the Check for Differences is expected. I approve this PR. Just a reminder to create a new Update Truth METplus issue to describe the changes to existing METplus Use Cases.

@hsoh-u hsoh-u merged commit eb35a2e into main_v11.1 May 3, 2024
35 of 37 checks passed
@JohnHalleyGotway JohnHalleyGotway removed their request for review May 6, 2024 14:55
JohnHalleyGotway added a commit that referenced this pull request May 6, 2024
* Add user execute permissions to compile script - main_v11.1 (#2740)

* Changing -j to "-j 5" as the recommended value for MAKE_ARGS

* Per #2761, update the MET development environment after upgrading seneca to debian bookworm.

* Per #2761, define runtime python version for testing  rather than using the default version which no longer exists in /usr/local

* Per #2761, fix  setting ci-skip-all

* Per #2761, patching test_util.R to use the -C command line option for ncdiff.

* #2652 Added find_var_by_standard_name and separated common codes to find_xy_vars

* #2757 The SonarQube token and URL are replaced with the pre-defined strings, SONAR_TOKEN_VALUE and SONAR_SERVER_URL

* #2757 Get the email list from the environment variable MET_CRON_EMAIL_LIST__MET (or MET_CRON_EMAIL_LIST)

* #2757 The SonarQube token and URL are replaced by using the environment variable SONAR_TOKEN_VALUE and SONAR_SERVER_URL

* Bugfix #2760 main_v11.1 --enable-python (#2767)

* #2755 Added a header count and checking header count instead of using header id (hid)

* Bugfix #2782 main_v11.1 MASSDEN (#2784)

* Per #2782, update the multiple matching records warning message to include the table number for each record.

* Per #2782, update read_grib2_record_list() to parse the level values and aerosol information correctly for table 4.48.

* Per #2782 tweak variable naming convention.

* Removing ${MAKE_ARGS} in some locations

Removing ${MAKE_ARGS} from "make install" and "make test" for MET.  Removing "met" prefix from met.configure.log because we really need config.log for any detail.  It is confusing to have met.configure.log when that does not contain useful information.

* Adding -lnetcdf -lm to configure_lib_args for NetCDF-CXX

* Feature #2796 main_v11.1 gha node20 (#2798)

* Per #2796, update versions of actions to fix the node 16 to 20 warning message.

* Per #2796, port fixes for artifact name handling over from the develop branch to the main_v11.1 testing workflow. Also add the compilation_options.yml workflow since the workflows are being updated.

* Create 11.1.0_casper

* Recent changes to branch protection rules for the main_vX.Y branches have broken the logic of the update_truth.yml GHA workflow. Instead of submitting a PR to merge main_vX.Y into main_vX.Y-ref directly, use an intermediate update_truth_for_main_vX.Y branch.

* Update the pull request template to include a question about expected impacts to existing METplus Use Cases.

* Bugfix #2833 main_v11.1 azimuth (#2834)

* Per #2833, fix n-1 bug when defining the azimuth delta for range/azimuth grids.

* Per #2833, port fixes over from the bugfix_2833_develop_azimuth branch over to the main_v11.1 branch.

---------

Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>

* Feature #2379 main_v11.1 sonarqube GHA (#2848)

* Per #2379, migrating largely the same changes for #2379 into the main_v11.1 branch. The difference is that --enable-all is not used since that is not a valid configuration option for MET version 11.1.0.

* Hotfix related to #2379. The sonar.newCode.referenceBranch and sonar.branch.name cannot be set to the same string! Only add the newCode definition when they differ.

* Feature #2379 main_v11.1 sonarqube updates (#2851)

* Feature #2379 main_v11.1 single_sq_project (#2866)

* Bugfix 2867 point2grid qc flag main v11.1 (#2874)

* #2867 Added point2grid_GOES_16_ADP_Enterprise_high and changed qc flags for ADP

* #2867 Added get_nc_att_values_

* #2867 Added get_nc_att_values

* #2867 Get the ADP QC flag values from the varibale attributes (support GOES16 Enterprise allgorithm) and apply them for QC Flags. Adjusted the ADP QC flags based on the variable QC values

---------

Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>

---------

Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com>
Co-authored-by: Julie Prestopnik <jpresto@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>
Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>
Co-authored-by: Howard Soh <hsoh@ucar.edu>
Co-authored-by: metplus-bot <97135045+metplus-bot@users.noreply.github.com>
JohnHalleyGotway added a commit that referenced this pull request Jun 25, 2024
* Add user execute permissions to compile script - main_v11.1 (#2740)

* Changing -j to "-j 5" as the recommended value for MAKE_ARGS

* Per #2761, update the MET development environment after upgrading seneca to debian bookworm.

* Per #2761, define runtime python version for testing  rather than using the default version which no longer exists in /usr/local

* Per #2761, fix  setting ci-skip-all

* Per #2761, patching test_util.R to use the -C command line option for ncdiff.

* #2652 Added find_var_by_standard_name and separated common codes to find_xy_vars

* #2757 The SonarQube token and URL are replaced with the pre-defined strings, SONAR_TOKEN_VALUE and SONAR_SERVER_URL

* #2757 Get the email list from the environment variable MET_CRON_EMAIL_LIST__MET (or MET_CRON_EMAIL_LIST)

* #2757 The SonarQube token and URL are replaced by using the environment variable SONAR_TOKEN_VALUE and SONAR_SERVER_URL

* Bugfix #2760 main_v11.1 --enable-python (#2767)

* #2755 Added a header count and checking header count instead of using header id (hid)

* Bugfix #2782 main_v11.1 MASSDEN (#2784)

* Per #2782, update the multiple matching records warning message to include the table number for each record.

* Per #2782, update read_grib2_record_list() to parse the level values and aerosol information correctly for table 4.48.

* Per #2782 tweak variable naming convention.

* Removing ${MAKE_ARGS} in some locations

Removing ${MAKE_ARGS} from "make install" and "make test" for MET.  Removing "met" prefix from met.configure.log because we really need config.log for any detail.  It is confusing to have met.configure.log when that does not contain useful information.

* Adding -lnetcdf -lm to configure_lib_args for NetCDF-CXX

* Feature #2796 main_v11.1 gha node20 (#2798)

* Per #2796, update versions of actions to fix the node 16 to 20 warning message.

* Per #2796, port fixes for artifact name handling over from the develop branch to the main_v11.1 testing workflow. Also add the compilation_options.yml workflow since the workflows are being updated.

* Create 11.1.0_casper

* Recent changes to branch protection rules for the main_vX.Y branches have broken the logic of the update_truth.yml GHA workflow. Instead of submitting a PR to merge main_vX.Y into main_vX.Y-ref directly, use an intermediate update_truth_for_main_vX.Y branch.

* Update the pull request template to include a question about expected impacts to existing METplus Use Cases.

* Bugfix #2833 main_v11.1 azimuth (#2834)

* Per #2833, fix n-1 bug when defining the azimuth delta for range/azimuth grids.

* Per #2833, port fixes over from the bugfix_2833_develop_azimuth branch over to the main_v11.1 branch.

---------

Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>

* Feature #2379 main_v11.1 sonarqube GHA (#2848)

* Per #2379, migrating largely the same changes for #2379 into the main_v11.1 branch. The difference is that --enable-all is not used since that is not a valid configuration option for MET version 11.1.0.

* Hotfix related to #2379. The sonar.newCode.referenceBranch and sonar.branch.name cannot be set to the same string! Only add the newCode definition when they differ.

* Feature #2379 main_v11.1 sonarqube updates (#2851)

* Feature #2379 main_v11.1 single_sq_project (#2866)

* Bugfix 2867 point2grid qc flag main v11.1 (#2874)

* #2867 Added point2grid_GOES_16_ADP_Enterprise_high and changed qc flags for ADP

* #2867 Added get_nc_att_values_

* #2867 Added get_nc_att_values

* #2867 Get the ADP QC flag values from the varibale attributes (support GOES16 Enterprise allgorithm) and apply them for QC Flags. Adjusted the ADP QC flags based on the variable QC values

---------

Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>

* Bugfix 2867 point2grid qc flag main v11.1 (#2878)

* #2867 Added point2grid_GOES_16_ADP_Enterprise_high and changed qc flags for ADP

* #2867 Added get_nc_att_values_

* #2867 Added get_nc_att_values

* #2867 Get the ADP QC flag values from the varibale attributes (support GOES16 Enterprise allgorithm) and apply them for QC Flags. Adjusted the ADP QC flags based on the variable QC values

* #2867 Added adjusted confidnece counts

* #2867 Corretced indent

* #2867 Corretced indent

---------

Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>

* #2884 No filtering by QC flags without -qc option (#2885)

Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>

* Per #2659, making updates as proposed at the 20240516 MET Eng. Mtg. (#2896)

* Bugfix #2897 main_v11.1 python_valid_time (#2898)

* Per #2897, fix typos in 3 log messages. Also fix the bug in storing the valid time strings. The time string in vld_array should exactly correspond to the numeric unixtime values in vld_num_array. Therefore they need to be updated inside the same if block. The bug is that we were storing only the unique unixtime values but storing ALL of the valid time string, not just the unique ones.

* Per #2897, don’t waste time searching, just set vld_odd to n-1

* #2687 Saved the PBL input into the vector (#2903)

* #2904 Changed R path to R-4.4.0 (#2909)

Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>

* Bugfix #2856 main_v11.1 ens_climo (#2917)

* Per #2856, reinitialize the climo cdf info pointer.

* Per #2856, update the testing.yml workflow dispatch option to let the comparison branch be manually defined.

* Revert "Per #2856, reinitialize the climo cdf info pointer."

This reverts commit 99b6bb3.

* Per #2856, reinitialize the climo cdf info pointer.

* Revert "Per #2856, update the testing.yml workflow dispatch option to let the comparison branch be manually defined."

This reverts commit 0ae224f.

* Per #2856, update the truth_data_version and input_data_version if main_vX.Y is not present in the branch name.

* Update set_job_controls.sh

* Update set_job_controls.sh

* Per #2856, update logic for counting the maximum number of PCT, PRC, PJC, and PSTD lines. Need to multiply by the number of climo cdf bins.

* Per #2856, add more error checking to avoid writing nan to .stat output files

* Bugfix #2841 main_v11.1 tang_rad_winds (#2920)

* Per #2841, unrelated, just removing a spurious character from a  log message.

* Per #2841, work in progress. Mostly just modifying whitespace and log messages so far

* Per #2841, running tc_rmw with only UGRD input causes the tool to hang. Adding break statements to prevent the hang.

* Per #2841, add new has_pressure_level() utility function and update tc_rmw to use it to deteremine whether or not the pressure dimension should be written the output rather than basing that off the number of levels being > 1.

* Per #1849, fix to radial and tangential winds.

* Per #2841, just changing a code comment about the longitude swap

* Per #2841, update variable names for consistency and understanding.

* Per #2841, the substantive bug fixed here is how we index into the U and V data in wind_ne_to_rt() using i_rev instead of i. Also, update the variable names for consistency and clarity..

* Per #2841, patch apparent copy/paste bug in tcrmw_grid.cc code.

* Per #2841, only changing whitespace.

* Per #2841, update logic in EarthRotation::set_tcrmw() to change the rotation of TCRMW grids from pointing north to pointing east.

* Per #2841, fix warning message

* Replace tab with spaces

* Per #2841, correct the units for the azimuth netcdf output variable

* Per #2841, reverse the x dimension of the rotated latlon grid to effectively switch from counterclockwise rotation to clockwise.

* Feature #2855 v11.1.1 (#2923)

* Per #2855, add v11.1.1 release notes.

* Per #2841, fix bold formatting of release notes.

* Update conf.py with actual 11.1.1 release date

---------

Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com>
Co-authored-by: Julie Prestopnik <jpresto@ucar.edu>
Co-authored-by: John Halley Gotway <johnhg@ucar.edu>
Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu>
Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu>
Co-authored-by: Howard Soh <hsoh@ucar.edu>
Co-authored-by: metplus-bot <97135045+metplus-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
3 participants