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.

Sign up for GitHub

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

Jump to bottom

Updating High Resolution Tests, Fixing Minor Wave Issues #3289

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Updating High Resolution Tests, Fixing Minor Wave Issues #3289

wants to merge 16 commits into from

Conversation

JessicaMeixner-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA commented Jan 30, 2025 •
edited
Loading

Description

This PR updates the wave fix which was unintentionally left out of PR #3190
Additionally, this updates what ICs the highres ci tests are pointing to (IC update issues: #3287 #3288)

-- Fixes #3260

Additionally, some smaller bug fixes and changes were made:

Notes on ufs-weather-model updates:

Commit date Commit hash/ PR Notes for g-w changes Baseline Changes
Jan 7, 2025 ufs-community/ufs-weather-model@5324d64 ufs-community/ufs-weather-model#2531 n/a changes in regional tests, no global changes
Jan 13, 2025 ufs-community/ufs-weather-model@c7412fa n/a changes in some global coupled tests
Jan 17, 2025 ufs-community/ufs-weather-model@3a5e52e n/a no changes
Jan 21, 2025 ufs-community/ufs-weather-model@33cde4b extra variable needed for waves no changes
Jan 22, 2025 ufs-community/ufs-weather-model@b9d2bec n/a no changes
Jan 28, 2025 ufs-community/ufs-weather-model@0682f90 n/a
Feb 3, 2025 ufs-community/ufs-weather-model@a498735 add fv3atm_output_dir to model_configure answers change
Feb 5, 2025 ufs-community/ufs-weather-model@ee3d52d changes in this PR for initialization for wave fixes if using WW3 binary restart, answers will change

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO

While the full fix for the wave IC issue does require code changes, these changes are still needed and can be included before the WW3 update is there.

How has this been tested?

  • Ran a C48 S2SW with the WW3 code update: /scratch1/NCEPDEV/climate/Jessica.Meixner/UpdateFixandHiresIC/c48t05 (Also earlier ran ci/cases/hires/C768_S2SW.yaml which ran out of time in the forecast but otherwise was succeeding: /scratch1/NCEPDEV/climate/Jessica.Meixner/UpdateFixandHiresIC/c768t01 )
  • hires/C768_S2SW.yaml on orion is in progress: /work2/noaa/marine/jmeixner/UpdateFixandHiresIC/t01c768
  • hires/C1152_S2SW.yaml on hercules is in progress: /work2/noaa/marine/jmeixner/hercules/UpdateFixandHiresIC/t01c1152

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 30, 2025
View reviewed changes
jobs/JGLOBAL_WAVE_POST_SBS Fixed Show resolved Hide resolved
This was referenced Jan 30, 2025
Open
Open
WalterKolczynski-NOAA
WalterKolczynski-NOAA previously approved these changes Feb 3, 2025
View reviewed changes
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

LGTM

@WalterKolczynski-NOAA
Copy link
Contributor

Hold CI until the new ICs gets staged

sbanihash
sbanihash reviewed Feb 3, 2025
View reviewed changes
Copy link
Contributor

@sbanihash sbanihash left a comment

Choose a reason for hiding this comment

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

changes in ush/forecast_postdet.sh look good to me. I did a test only with this change and the restart reading is working as expected. I have not tested changes within the post job.

@sbanihash sbanihash mentioned this pull request Feb 3, 2025
Merged
4 tasks
aerorahul
aerorahul previously approved these changes Feb 4, 2025
View reviewed changes
Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

lgtm

@WalterKolczynski-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA would you please merge in develop before we start CI? There were a couple issues that were corrected yesterday that I want to capture.

@JessicaMeixner-NOAA
Copy link
Contributor Author

@WalterKolczynski-NOAA - Merged develop.

@WalterKolczynski-NOAA WalterKolczynski-NOAA added the CI-Hera-Ready **CM use only** PR is ready for CI testing on Hera label Feb 5, 2025
@emcbot emcbot added CI-Hera-Building **Bot use only** CI testing is cloning/building on Hera and removed CI-Hera-Ready **CM use only** PR is ready for CI testing on Hera CI-Hera-Building **Bot use only** CI testing is cloning/building on Hera labels Feb 5, 2025
@JessicaMeixner-NOAA
Copy link
Contributor Author

Yeah - sorry, I'll make sure I can run something before this. I thought I had run things close enough and looked at every commit for needed changes but missed this. Apologies for not properly testing again on my end. I'll do that now.

@KateFriedman-NOAA
Copy link
Member

Okie dokie, I will change the label to failed for WCOSS2. Please reset it to WCOSS2-ready and ping me when ready to retry the CI tests. Thanks!

@KateFriedman-NOAA KateFriedman-NOAA added CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed and removed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress labels Feb 7, 2025
@aerorahul
Copy link
Contributor

@aerorahul the unbound variable is: /lfs/h2/emc/ptmp/emc.global/PR/PR_3289/global-workflow/ush/atparse.bash: line 82: FV3ATM_OUTPUT_DIR: unbound variable

I missed we need to add FV3ATM_OUTPUT_DIR here. I'll run a test and let you know when it's passed. @aerorahul - I'm assuming for now this is just "./" and then another PR will update the configuration and use of this later?

Adding

local FV3ATM_OUTPUT_DIR="./"

in ush/parsing_model_configure_FV3.sh would address this unbound error issue. The path will be updated in a future PR.

@JessicaMeixner-NOAA
Copy link
Contributor Author

My test is now in the forecast job
/work2/noaa/marine/jmeixner/hercules/updateufshash/t01/COMROOT/t01/logs/2021032312/gfs_fcst_seg0.log
and running.

@JessicaMeixner-NOAA JessicaMeixner-NOAA added CI-Wcoss2-Ready **CM use only** PR is ready for CI testing on WCOSS and removed CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed labels Feb 7, 2025
@KateFriedman-NOAA KateFriedman-NOAA added CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress and removed CI-Wcoss2-Ready **CM use only** PR is ready for CI testing on WCOSS CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS labels Feb 7, 2025
@KateFriedman-NOAA
Copy link
Member

CI Tests set up to run in /lfs/h2/emc/ptmp/emc.global/PR/PR_3289/RUNTESTS on WCOSS

@KateFriedman-NOAA
Copy link
Member

Execs weren't built in second attempt at CI. Retrying CI.

@KateFriedman-NOAA KateFriedman-NOAA added CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress and removed CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS labels Feb 7, 2025
@KateFriedman-NOAA
Copy link
Member

CI Tests set up to run in /lfs/h2/emc/ptmp/emc.global/PR/PR_3289/RUNTESTS on WCOSS

@KateFriedman-NOAA KateFriedman-NOAA added CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress and removed CI-Wcoss2-Failed **Bot use only** CI testing on WCOSS for this PR has failed CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress CI-Wcoss2-Building **Bot use only** CI testing is cloning/building on WCOSS labels Feb 7, 2025
@KateFriedman-NOAA
Copy link
Member

CI Tests set up to run in /lfs/h2/emc/ptmp/emc.global/PR/PR_3289/RUNTESTS on WCOSS

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

@WalterKolczynski-NOAA WalterKolczynski-NOAA WalterKolczynski-NOAA left review comments

@sbanihash sbanihash sbanihash left review comments

@aerorahul aerorahul aerorahul left review comments

@junwang-noaa junwang-noaa Awaiting requested review from junwang-noaa junwang-noaa is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
CI-Wcoss2-Running **Bot use only** CI testing on WCOSS for this PR is in-progress
Projects
None yet
Milestone
No milestone
6 participants
@JessicaMeixner-NOAA @WalterKolczynski-NOAA @emcbot @KateFriedman-NOAA @aerorahul @sbanihash