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 #2437 main_v11.0 convert #2438

Merged
merged 11 commits into from
Feb 15, 2023

Conversation

JohnHalleyGotway
Copy link
Collaborator

@JohnHalleyGotway JohnHalleyGotway commented Feb 6, 2023

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:

Manually tested using a single monthly input file.

The differences flagged in the GHA run for this PR are the addition of 4 new output files, as expected:

ERROR: folder /data/output/met_test_truth missing 4 files
    climatology_2.5deg/grid_stat_SINGLE_MONTH_CLIMO_2.5DEG_000000L_20201225_120000V.stat
    climatology_2.5deg/grid_stat_SINGLE_MONTH_CLIMO_2.5DEG_000000L_20201225_120000V_pairs.nc
    climatology_2.5deg/grid_stat_WRAP_YEAR_CLIMO_2.5DEG_000000L_20201225_120000V.stat
    climatology_2.5deg/grid_stat_WRAP_YEAR_CLIMO_2.5DEG_000000L_20201225_120000V_pairs.nc

And changing the FCST_UNITS and OBS_UNITS in existing output files is also flagged, as expected:

ERROR: header information mismatch in column FCST_UNITS
  • Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:

You can test these code on seneca as the met_test user in:
/d1/projects/MET/MET_pull_requests/met-11.0.1/issue_2437/MET-main_v11.0_2437_develop_convert

You could test additional dates, including leap years, and pay close attention to the "Skipping", "Storing", "Shifting", and "Interpolating" log messages to confirm that the logic is working as expected.

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

  • Do these changes include sufficient testing updates? [Yes]

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

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

Adds a new call to Grid-Stat to run an existing unit test but with 1 monthly climo input time instead of 2.

  • Please complete this pull request review by [Thurs 2/9/23].

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 Development issue 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.

…dding logic. This was the source of the climo conversion logic being applied twice. It was applied when reading the climo data, and then again after regridding it to the verification domain. The convert() and censor() options should only be applied when READING data, not when processing it. Run a full regression test to make sure this change does not break any unit tests ci-run-unit
…e RegridInfo struct in #1293. Instead of removing it entirely, I'll try to limit the scope of parsing those entries to just the regrid dictionary WITHOUT doing any lookup in its parents.
…up functions in the Dictionary class. This will enable us to control whether or not the higher levels of config file context should be searched. In the case of regrid.convert(x), regrid.censor_thresh, and regrid.censor_val, the parents should NOT be searched to avoid their double application. Running the full regression test to see if this change breaks anything.'
…files. I also checked METplus and do not see it in the wrapped Grid-Stat config file.
…2 climo records are found but the target valid time does not fall between them. Instead of erroring out, update the climo times to match the date (year, month, day) of the valid time. Then proceed with the interpolation logic.
…ttr_units option. Port over existing Grid-Stat unit test to demonstrate wrapping the year. And add another Grid-Stat unit test to use only a single monthly input climo file.
… needed and refine the logic for updating the climo timestamps by shifting them a fixed number of days.
Copy link
Contributor

@j-opatz j-opatz left a comment

Choose a reason for hiding this comment

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

As updated in the develop PR, this PR will still experience issues for date interpolation, but not because of the coding. Rather, the data being tested has irregularities for dates on the 18z variables.
In the same way the develop bugfix was approved, this PR is also approved as it meets the requested outcomes of the bugfix issue.

@JohnHalleyGotway JohnHalleyGotway merged commit 245ebe9 into main_v11.0 Feb 15, 2023
@JohnHalleyGotway JohnHalleyGotway deleted the bugfix_2437_main_v11.0_convert branch February 15, 2023 19:43
JohnHalleyGotway added a commit that referenced this pull request Feb 16, 2023
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
JohnHalleyGotway added a commit that referenced this pull request Feb 22, 2023
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
fix 2428 python from env main v11.0 (#2443)
fix 2428 python csv input (#2450)
fix #2452 main_v11.0 airnow (#2453)
fix #2402 main_v11.0 sonarqube (First PR) (#2447)
JohnHalleyGotway added a commit that referenced this pull request Mar 31, 2023
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
fix 2428 python from env main v11.0 (#2443)
fix 2428 python csv input (#2450)
fix #2452 main_v11.0 airnow (#2453)
fix #2402 main_v11.0 sonarqube (First PR) (#2447)
fix #2449 main_v11.0 pdf (#2465)
fix 2428 python csv input (#2467)
JohnHalleyGotway added a commit that referenced this pull request Apr 3, 2023
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
fix 2428 python from env main v11.0 (#2443)
fix 2428 python csv input (#2450)
fix #2452 main_v11.0 airnow (#2453)
fix #2402 main_v11.0 sonarqube (First PR) (#2447)
fix #2449 main_v11.0 pdf (#2465)
fix 2428 python csv input (#2467)
JohnHalleyGotway pushed a commit that referenced this pull request Sep 11, 2023
Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
Co-authored-by: lisagoodrich <[email protected]>
Co-authored-by: Stephen Herbener <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
fix 2428 python from env main v11.0 (#2443)
fix 2428 python csv input (#2450)
fix #2452 main_v11.0 airnow (#2453)
fix #2402 main_v11.0 sonarqube (First PR) (#2447)
fix #2449 main_v11.0 pdf (#2465)
fix 2428 python csv input (#2467)
fix #2514 main_v11.0 clang (#2556)
fix #2575 main_v11.0 python_convert (#2577)
fix 2596 main v11.0 rpath compilation (#2600)
fix 2596 main v11.0 rpath compilation (#2613)
fix #2644 main_v11.0 percentile (#2645)
JohnHalleyGotway added a commit that referenced this pull request Dec 18, 2023
…2770)

Co-authored-by: MET Tools Test Account <[email protected]>
Co-authored-by: Howard Soh <[email protected]>
Co-authored-by: John Halley Gotway <[email protected]>
Co-authored-by: jprestop <[email protected]>
Co-authored-by: hsoh-u <[email protected]>
Co-authored-by: George McCabe <[email protected]>
Co-authored-by: lisagoodrich <[email protected]>
Co-authored-by: Stephen Herbener <[email protected]>
fix #2389 main_v11.0 flowchart (#2391)
fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405)
fix #2380 main_v11.0 override (#2381)
fix #2408 main_v11.0 empty config (#2409)
fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403)
fix #2415 main_v11.0 modulefiles (#2416)
fix #2412 main_v11.0 climo (#2420)
fix #2426 main_v11.0 buoy (#2432)
fix #2437 main_v11.0 convert (#2438)
fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup.
fix 2428 python from env main v11.0 (#2443)
fix 2428 python csv input (#2450)
fix #2452 main_v11.0 airnow (#2453)
fix #2402 main_v11.0 sonarqube (First PR) (#2447)
fix #2449 main_v11.0 pdf (#2465)
fix 2428 python csv input (#2467)
fix #2514 main_v11.0 clang (#2556)
fix #2575 main_v11.0 python_convert (#2577)
fix 2596 main v11.0 rpath compilation (#2600)
fix 2596 main v11.0 rpath compilation (#2613)
fix #2644 main_v11.0 percentile (#2645)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
2 participants