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

Update ufs-weather-model to 2024-12-06 commit #3145

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

Conversation

JessicaMeixner-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA commented Dec 6, 2024

Description

This PR updates ufs-weather-model hash to the latest commit which addresses an issue seen in an earlier commit (see: #3110)

Note this includes the PR that enables PIO for WW3, however additional work is required to use this feature.
Another notable PR update is for a PIO finalize bug in CICE which also required updates to ice_in

Resolves #3110
Resolves #3121

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 (If YES, please add a link to any PRs that are pending.)
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

Quick test that forecast runs for S2SW GFS. Running a quick cycled test but wanted to share this now so others don't have to make the same updates here in their local branches.

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

aerorahul
aerorahul previously approved these changes Dec 6, 2024
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.

looks straight-forward.

DavidHuber-NOAA
DavidHuber-NOAA previously approved these changes Dec 9, 2024
@DavidHuber-NOAA DavidHuber-NOAA added the CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules label Dec 9, 2024
@emcbot emcbot added CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress and removed CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules labels Dec 9, 2024
@JessicaMeixner-NOAA
Copy link
Contributor Author

Sorry I just was checking some tests I ran over the weekend for the WCDA - and I found an issue so we might want to stop the CI as I'm assuming itll find the same issue!

For the WCDA test:

The log file is here:
/scratch1/NCEPDEV/climate/Jessica.Meixner/gwprupdateufs/twcda01/COMROOT/twcda01/logs/2021032412/gdas_fcst_seg0.log.0 and we get an error:

20:
20:  (abort_ice)ABORTED:
20:  (abort_ice) error =
20:  (construct_filename) ERROR: history filename already used for another history s
20:  tream iceh_inst.2021-03-24-43200.nc

The ice_in file is here: /scratch1/NCEPDEV/stmp2/Jessica.Meixner/RUNDIRS/twcda01/gdas.2021032412/gdasfcst.2021032412/fcst.548096/ice_in

There is one difference between GDAS and GFS runs:
https://github.com/NOAA-EMC/global-workflow/blob/develop/ush/parsing_namelists_CICE.sh#L74-L78

So I don't know if maybe the new CICE_hist_suffix should also have a GFS/GDAS setting? @DeniseWorthen @NickSzapiro-NOAA any chance you might have insight to this?

@emcbot emcbot added CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed and removed CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress labels Dec 9, 2024
@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C96_atm3DVar FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C96_atm3DVar_8020fc79

@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C48_S2SWA_gefs FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C48_S2SWA_gefs_8020fc79

@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C96_S2SWA_gefs_replay_ics FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C96_S2SWA_gefs_replay_ics_8020fc79

@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C48_ATM FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C48_ATM_8020fc79

@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C96C48_hybatmDA FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C96C48_hybatmDA_8020fc79

@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C48_S2SW FAILED on Hercules in Build# 1 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C48_S2SW_8020fc79

@emcbot emcbot added CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed and removed CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed labels Dec 9, 2024
@emcbot
Copy link

emcbot commented Dec 9, 2024

Experiment C96C48_hybatmDA FAILED on Hercules in Build# 2 in
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C96C48_hybatmDA_e3a1bd94

@emcbot emcbot added CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed and removed CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed labels Dec 10, 2024
@emcbot
Copy link

emcbot commented Dec 10, 2024

CI Failed on Hercules in Build# 2
Built and ran in directory /work2/noaa/global/CI/HERCULES/3145


Experiment C96_atm3DVar_e3a1bd94 Terminated with 0
FAIL
FAIL tasks failed and 2 dead at Mon Dec  9 15:21:53 CST 2024
Experiment C96_atm3DVar_e3a1bd94 Terminated: *FAIL*
Error logs:
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/COMROOT/C96_atm3DVar_e3a1bd94/logs/2021122100/gdas_atmanlupp.log
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/COMROOT/C96_atm3DVar_e3a1bd94/logs/2021122100/gfs_atmanlupp.log
Experiment C96C48_hybatmDA_e3a1bd94 Terminated with 0
FAIL
FAIL tasks failed and 1 dead at Mon Dec  9 15:40:05 CST 2024
Experiment C96C48_hybatmDA_e3a1bd94 Terminated: *FAIL*
Error logs:
/work2/noaa/global/CI/HERCULES/3145/RUNTESTS/COMROOT/C96C48_hybatmDA_e3a1bd94/logs/2021122100/gfs_atmanlupp.log
Experiment C48_ATM_e3a1bd94 Completed 2 Cycles: *SUCCESS* at Mon Dec  9 15:46:00 CST 2024
Experiment C96_S2SWA_gefs_replay_ics_e3a1bd94 Completed 1 Cycles: *SUCCESS* at Mon Dec  9 17:47:58 CST 2024
Experiment C48_S2SW_e3a1bd94 Completed 2 Cycles: *SUCCESS* at Mon Dec  9 18:30:28 CST 2024
Experiment C48_S2SWA_gefs_e3a1bd94 Completed 1 Cycles: *SUCCESS* at Mon Dec  9 22:51:07 CST 2024

@DavidHuber-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA @aerorahul The nam_micro_lookup.dat file was moved in the UPP from the parm/ directory to fix/, so link_workflow.sh needs to be updated. See commit NOAA-EMC/UPP@a6c1a38. Should this be moved to fix/ in the global workflow?

@JessicaMeixner-NOAA
Copy link
Contributor Author

Thanks @DavidHuber-NOAA - It does indeed look like we need some updates. @WenMeng-NOAA can you confirm these changes needed for UPP? I'll start making an update now.

sorc/link_workflow.sh Fixed Show fixed Hide fixed
sorc/link_workflow.sh Fixed Show fixed Hide fixed
@JessicaMeixner-NOAA
Copy link
Contributor Author

I've updated the link script and the gdas_atmanlupp job succeeded. My archive failed because I forgot to switch from emc-global to an account I have access to.

@aerorahul
Copy link
Contributor

Thanks @DavidHuber-NOAA - It does indeed look like we need some updates. @WenMeng-NOAA can you confirm these changes needed for UPP? I'll start making an update now.

@JessicaMeixner-NOAA I can confirm these changes are required for the UPP. They performed some house cleanup and moved files from parm/ to fix/.
Thanks for including this necessary update

@JessicaMeixner-NOAA
Copy link
Contributor Author

I'm getting shell check warnings - let me know if you would like to see any changes @aerorahul @DavidHuber-NOAA

do
${LINK_OR_COPY} "${HOMEgfs}/sorc/upp.fd/parm/${file}" .
done
cd "${HOMEgfs}/parm/post" || exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are already in this directory. L147

sorc/link_workflow.sh Outdated Show resolved Hide resolved
@WalterKolczynski-NOAA
Copy link
Contributor

I'm getting shell check warnings - let me know if you would like to see any changes @aerorahul @DavidHuber-NOAA

You should resolve the shellcheck warnings. Looks like you just need to get rid of the one-element loops.

@WalterKolczynski-NOAA
Copy link
Contributor

NB: ufs-community/ufs-weather-model/issues/2502 wasn't closed when the fix was merged into UFS because the original PR got merged into another PR that was not updated to reflect the issue resolution.

@WalterKolczynski-NOAA WalterKolczynski-NOAA changed the title Update ufs-weather-model to 12/6/2024 commit Update ufs-weather-model to 2024-12-06 commit Dec 10, 2024
@WalterKolczynski-NOAA WalterKolczynski-NOAA added CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules and removed CI-Hercules-Failed **Bot use only** CI testing on Hercules for this PR has failed labels Dec 10, 2024
@emcbot emcbot added CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules and removed CI-Hercules-Ready **CM use only** PR is ready for CI testing on Hercules labels Dec 10, 2024
@WenMeng-NOAA
Copy link
Contributor

Thanks @DavidHuber-NOAA - It does indeed look like we need some updates. @WenMeng-NOAA can you confirm these changes needed for UPP? I'll start making an update now.

@JessicaMeixner-NOAA The UPP related updates in link_workflow.sh look good to me. Thanks!

@emcbot emcbot added CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress and removed CI-Hercules-Building **Bot use only** CI testing is cloning/building on Hercules labels Dec 10, 2024
@TerrenceMcGuinness-NOAA
Copy link
Collaborator

TerrenceMcGuinness-NOAA commented Dec 11, 2024

Gather Rocoto statistics
(C96_S2SWA_gefs_replay_ics_32ec7b29 on Hercules)
	Total Cycles: 1
	Number Cycles done: 1
	State: DONE
Experiment C96_S2SWA_gefs_replay_ics_32ec7b29 Completed 1 Cycles: *SUCCESS* at Tue Dec 10 22:46:56 CST 2024

CI Experiment passed for case C96_S2SWA_gefs_replay_ics for this PR but the Jenkins failed on remote clean up.

/work2/noaa/global/CI/HERCULES/3145/gfs/ci/scripts/utils/ci_utils_wrapper.sh cleanup_experiment /work2/noaa/global/CI/HERCULES/3145/RUNTESTS/EXPDIR/C96_S2SWA_gefs_replay_ics_32ec7b29
Shell Script 2.3 sec Found unhandled java.io.IOException exception: Remote call on Hercules-EMC failed

Experiment C48_ATM_32ec7b29 Completed 2 Cycles: SUCCESS at Tue Dec 10 16:29:45 CST 2024
Experiment C96_atm3DVar_32ec7b29 Completed 3 Cycles: SUCCESS at Tue Dec 10 19:00:57 CST 2024
Experiment C96C48_hybatmDA_32ec7b29 Completed 3 Cycles: SUCCESS at Tue Dec 10 19:01:13 CST 2024
Experiment C96_S2SWA_gefs_replay_ics_32ec7b29 Completed 1 Cycles: SUCCESS at Tue Dec 10 22:46:56 CST 2024

The remaining two cases C48_S2SWA_gefs and C48_S2SW are waiting in the Priority Queue

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

@TerrenceMcGuinness-NOAA
Copy link
Collaborator

@JessicaMeixner-NOAA This CI test is still running nominally ...

Experiment C48_ATM_32ec7b29 Completed 2 Cycles: SUCCESS at Tue Dec 10 16:29:45 CST 2024
Experiment C96_atm3DVar_32ec7b29 Completed 3 Cycles: SUCCESS at Tue Dec 10 19:00:57 CST 2024
Experiment C96C48_hybatmDA_32ec7b29 Completed 3 Cycles: SUCCESS at Tue Dec 10 19:01:13 CST 2024
Experiment C96_S2SWA_gefs_replay_ics_32ec7b29 Completed 1 Cycles: SUCCESS at Tue Dec 10 22:46:56 CST 2024

The two remaining cases C48_S2SW and C48_S2SWA_gefs are languishing in the Priority queues on Hercules

local WW3_user_histname="false"
local WW3_historync="false"
local WW3_restartnc="false"
local WW3_restart_from_binary="true"

Choose a reason for hiding this comment

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

The restart_from_binary option is only valid if you're also using restartnc. It allows you start from a binary but only write restarts in netcdf going forward.

If restartnc is false, then the option is not exercised.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The PIO restart and binary restarts are not yet being used. We are still using the inp file. @sbanihash is working on moving to nml and then we can turn on this feature. This was not done for this PR to expedite getting other fixes in.

Copy link

@DeniseWorthen DeniseWorthen Dec 12, 2024

Choose a reason for hiding this comment

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

I understand that, but you're setting the restart_from_binary=true, which implies you're asking the code to do something. But this setting is not pertinent w/o using restartnc. I would leave it false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-Hercules-Running **Bot use only** CI testing on Hercules for this PR is in-progress
Projects
None yet
10 participants