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

[develop] Bugfix bonanza #1108

Closed
wants to merge 30 commits into from

Conversation

gsketefian
Copy link
Collaborator

@gsketefian gsketefian commented Jul 22, 2024

DESCRIPTION OF CHANGES:

This PR fixes several bugs discovered in the verification portion of the app during recent testing by the DTC. The bugs are:

  1. Change to parm/data_locations.yml to account for the change in prebpufr (NDAS) obs file names on May 22, 2024. This is currently causing get_obs_ndas tasks to fail for cycles at or after this date. [Bug found by @michelleharrold.]

  2. Change to parm/wflow/verify_det.yaml so that all tasks have a cycldefs statement by default. This bug was causing GridStat workflow tasks for CCPA and NOHRSC obs to be created for cycles not defined for the workflow (these extraneous cycles probably correspond to the default set of cycles that a task gets assigned by ROCOTO when it does not contain an explicit cycledefs statement). [Bug found by @michelleharrold, solution by @mkavulich.]

  3. Change to scripts/exregional_run_met_gridstat_or_pointstat.sh to append a string for the cycle date ("_YYYYMDDHH") to the name of the metplus log file for deterministic GridStat and PointStat tasks. This was causing the metplus log file for GridStat for a given cycle tasks to be overwritten by those for other cycles. [Bug found by @michelleharrold.]

  4. Refactoring of scripts/exregional_get_verif_obs.sh to account for multiple overlapping cycles. It was found that when the experiment contains multiple cycles, the get_obs_ccpa, get_obs_mrms, and get_obs_ndas from the different cycles write to the same directories and clobber the output of other cycles or cause crashes (e.g. a directory is not empty when it is expected to be). [Bug found by @michelleharrold and @willmayfield.]. This script is refactored so that:
    a) "Raw" directories, i.e. the directories in which the data is pulled by the retrieve_data.py script, for each cycle or observation day are unique (contain either the cycle (raw_cycYYYYMMDDHH) or the date (raw_dayYYYYMMDD) in their names). Whether a cycle-based or obs-day-based raw directory is used depends on the obs type: CCPA raw directories are cycle-based, and MRMS and NDAS raw directories are day-based.
    b) There are checks on directories and files to make sure the get_obs_... task for another cycle is not writing to the directory and/or file the current cycle wishes to modify (write to, move, delete, etc).
    c) Data pulls and the final set of processed observations are organized so that there are no extra observation files that lie outside the range of the forecasts.
    d) Extra documentation is included in the script to describe in detail the behavior of the script for each obs type (CCPA, MRMS, NDAS).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TESTS CONDUCTED:

  • hera.intel
  • orion.intel
  • hercules.intel
  • cheyenne.intel
  • cheyenne.gnu
  • derecho.intel
  • gaea.intel
  • gaeac5.intel
  • jet.intel
  • wcoss2.intel
  • NOAA Cloud (indicate which platform)
  • Jenkins
  • fundamental test suite
  • comprehensive tests (specify which if a subset was used)

DOCUMENTATION:

ISSUE:

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

  • Work In Progress
  • bug
  • enhancement
  • documentation
  • release
  • high priority
  • run_ci
  • run_we2e_fundamental_tests
  • run_we2e_comprehensive_tests
  • Needs Cheyenne test
  • Needs Jet test
  • Needs Hera test
  • Needs Orion test
  • help wanted

CONTRIBUTORS (optional):

…the tar file where the prepbufr files live changed"
…y Michelle Harrold, solution by Michael Kavulich.
…ing cycles for CCPA and MRMS but not yet for NDAS or NOHRSC.
…thout performing unnecessary repeated pulls.
…files from HPSS (and works with multiple cycles).
…les, that are expected to be created once the task is finished actually get created. This is needed because it is possible that for some forecast hours for which there is overlap between cycles, the files are being retrieved and processed by the get_obs_... task for another cycle.
…nd EnsembleStat tasks such that GenEnsProd does not depend on the completion of get_obs_... tasks (because it doesn't need observations) but only forecast output while EnsembleStat does.
…d due to changes to dependencies of GenEnsProd tasks in previous commit(s).
…sure PcpCombine operates only on those hours unique to the cycle, i.e. for those times starting from the initial time of the cycle to just before the initial time of the next cycle. For the PcpCombine_obs task for the last cycle, allow it to operate on all hours of that cycle's forecast. This ensures that the PcpCombine tasks for the various cycles do not clobber each other's output. Accordingly, change the dependencies of downstream tasks that depend on PcpCombine obs output to make sure they include all PcpCombine_obs tasks that cover the forecast period of the that downstream task's cycle.
…ossibly also get_obs_ndas by putting in sleep commands.
@gsketefian
Copy link
Collaborator Author

@MichaelLueken I would like to close this draft PR and open a new one since I'm actually using a different branch for my work than bugfix/vx_bundle because the work I ended up doing is a lot more extensive than the initial intent of this PR. Is that ok with you?

@MichaelLueken
Copy link
Collaborator

@gsketefian - Sure, no problem. Please feel free to close this PR and open a new one from your new branch.

@gsketefian
Copy link
Collaborator Author

@MichaelLueken Great, thank you.

@gsketefian gsketefian closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants