Skip to content

Bugfix #3358 develop stageIV#3412

Draft
JohnHalleyGotway wants to merge 4 commits into
developfrom
bugfix_3358_develop_stageIV
Draft

Bugfix #3358 develop stageIV#3412
JohnHalleyGotway wants to merge 4 commits into
developfrom
bugfix_3358_develop_stageIV

Conversation

@JohnHalleyGotway

@JohnHalleyGotway JohnHalleyGotway commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Expected Differences

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

These changes include:

  • Adding 2 new calls to plot_data_plane to demonstrate reading this data.
  • Updating grid_base.cc to allow for Polar Stereographic grids to also be flipped from north to south.
  • Updating nc_cf_file.cc to do case insensitive unit checking, set data.scale_lat for Polar Stereo grids, and check for negative dy_km to set swap_to_north.

Note that I ran with both GRIB2 and NetCDF StageIV input data and confirmed that they both now log the same grid information:

DEBUG 4: Grid information:
DEBUG 4:    Projection: Stereographic
DEBUG 4:    Nx: 1121
DEBUG 4:    Ny: 881
DEBUG 4:    IsNorthHemisphere: true
DEBUG 4:    Lon_orient: 105.000
DEBUG 4:    Bx: 399.482
DEBUG 4:    By: 1599.405
DEBUG 4:    Alpha: 2496.3404
  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
    • Review code changes. I am a little concerned about this change:
-  data.scale_lat = proj_origin_lat;
+  data.scale_lat = proj_standard_parallel;

I'll note that proj_origin_lat is always set to +/-90 as defined in the CF-docs. But either standard_parallel or scale_factor_at_projection_origin can be specified, and I don't have examples of the latter with which to test out this logic.

  • Find the code available for testing in:
    seneca:/d1/projects/MET/MET_pull_requests/met-13.0.0/rc1/MET-bugfix_3358_develop_stageIV

  • Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
    None needed.

  • Do these changes include sufficient testing updates? [Yes]
    Added 2 new tests to unit_plot_data_plane.xml:

/d1/personal/johnhg/METplus/development/METplus-13.0/rc1/MET-bugfix_3358_develop_stageIV/internal/test_unit/../../share/met/../../bin/plot_data_plane \
      /d1/projects/MET/MET_test_data/unit_test/obs_data/stage4_nc/nws_precip_1day_20260410_conus.nc \
      /d1/personal/johnhg/METplus/development/METplus-13.0/rc1/MET-bugfix_3358_develop_stageIV/internal/test_unit/../../test_output/plot_data_plane/nws_precip_1day_20260410_conus.ps \
      'name="observation"; level="(*,*)"; censor_thresh=eq-10000; censor_val=-9999;' \
      -title "NWS Precip CONUS NetCDF-CF" \
      -v 1


/d1/personal/johnhg/METplus/development/METplus-13.0/rc1/MET-bugfix_3358_develop_stageIV/internal/test_unit/../../share/met/../../bin/plot_data_plane \
      /d1/projects/MET/MET_test_data/unit_test/obs_data/stage4_nc/nws_precip_1day_20260410_ak.nc \
      /d1/personal/johnhg/METplus/development/METplus-13.0/rc1/MET-bugfix_3358_develop_stageIV/internal/test_unit/../../test_output/plot_data_plane/nws_precip_1day_20260410_ak.ps \
      'name="observation"; level="(*,*)";' \
      -title "NWS Precip Alaska NetCDF-CF" \
      -v 1

Note the use of censor_thresh and censor_val since the NetCDF uses a data value of -10000 but does not specify it as being bad data.
Here are the resulting plots:
Screenshot 2026-07-08 at 4 56 22 PM

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

    Adds 2 new output files.

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

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

  • 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: METplus-X.Y Support project for bugfix releases or MET-X.Y Development project for the next coordinated release
  • 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.

…h as well since it shows up in NetCDF versions of StageIV data.
… of units and correct the logic for parsing the polar stereographic grid.
…using the min value to avoid anything unexpected.
@JohnHalleyGotway JohnHalleyGotway added this to the MET-13.0.0 milestone Jul 8, 2026
@JohnHalleyGotway
JohnHalleyGotway requested a review from hsoh-u July 8, 2026 22:56
@github-project-automation github-project-automation Bot moved this to 🩺 Needs Triage in METplus-13.0 Development Jul 8, 2026
@JohnHalleyGotway JohnHalleyGotway moved this from 🩺 Needs Triage to 🔎 In review in METplus-13.0 Development Jul 8, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
8.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@hsoh-u hsoh-u left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I approve. Thank you for adding the unit tests.

@JohnHalleyGotway
JohnHalleyGotway marked this pull request as draft July 9, 2026 16:28
@JohnHalleyGotway

Copy link
Copy Markdown
Collaborator Author

Converted this to a draft because the testing workflow for the PR flagged unexpected differences in two existing output files:

NSIDC0081_SEAICE_PS_N25km.ps
radolan_sp_PREC.ps

This warrants further investigation.

@JohnHalleyGotway

Copy link
Copy Markdown
Collaborator Author

This fix isn't working as I'd hoped. While the changes do produce desirable outputs for the CONUS and AK NWS data (as shown above), they "break" this existing plot of NSIDC0081_SEAICE_PS_N25km_20220723_v2.0.nc sea ice data for F16_ICECON.

Expected output:
Screenshot 2026-07-13 at 1 24 22 PM
With the changes for this PR:
Screenshot 2026-07-13 at 1 26 15 PM

The is the result of changes caused by dy_km < 0 which impacts which corner point is used to define the grid pin location, and the also swaps the data top to bottom.

I have not found a set of changes that handle both the existing NSIDC sea ice data and the NWS precip data properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 In review

Development

Successfully merging this pull request may close these issues.

Bugfix: Fix NetCDF CF convention support to read Stage IV precipitation files

2 participants