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

Tenth reconciliation PR from production/RRFS.v1 #2597

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

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented Feb 10, 2025

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

This work is from @Jili-DongNOAA and is identical to #2488:

This PR follows @JongilHan66 suggestion and aims to reduce the large convective reflectivity caused by saSAS adjustment in the first timestep during a warm start. The issue is likely related to the inconsistency when DA updates the moisture at t but not the moisture from the previous timestep (t-36s). The moisture from the previous timestep is needed for initializing sigmab (updraft area fraction) when calculating qadv (q advection or tendency term).

The PR forces qadv to zero in the first timestep when a namelist parameter sigmab_coldstart is set to .true. It also reduces the lower limit of sigmab from 0.01 to 0.0 in the first timestep.

The PR also resolves an issue found by @ericaligo-NOAA where inline post does not correctly output soil temperature and moisture in grib2 when using RUC LSM. We tracked this to post_fv3 where iSF_SURFACE_PHYSICS is not updated for RUC LSM. This parameter is used in UPP SURFACE.f for soil variables output. The initial iSF_SURFACE_PHYSICS = 2 in post_fv3 and should be updated to 3 when RUC LSM is used. In this PR we set iSF_SURFACE_PHYSICS =3 when nsoil=9 and successfully output the 9 level soil variables.

Commit Message:

* UFSWM - saSAS convective reflectivity fix; fix nsoil issue when using RUC LSM 
  * FV3 - saSAS convective reflectivity fix; fix nsoil issue when using RUC LSM
    * ccpp-physics - saSAS convective reflectivity fix; fix nsoil issue when using RUC LSM 

Priority:

  • High: SRW v3.0 release

Git Tracking

UFSWM:

  • None

Sub component Pull Requests:

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Updates/Changes Baselines.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • GaeaC5
    • GaeaC6
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@BrianCurtis-NOAA
Copy link
Collaborator

The year is 2026, and @grantfirl releases his 56th reconciliation PR from production/RRFS.v1

@grantfirl
Copy link
Collaborator Author

The year is 2026, and @grantfirl releases his 56th reconciliation PR from production/RRFS.v1

Ha. I hope this is the last one.

@grantfirl
Copy link
Collaborator Author

@JiliDong-NOAA You mentioned previously that this work would change tests using saSAS, but shouldn't it also change results for tests using RUC LSM? That is what I'm seeing so far when I'm running RTs on Hera. Please take a look at the test_changes.list file and verify that the answer changes are expected (when it is pushed in an hour or so).

@grantfirl grantfirl marked this pull request as ready for review February 10, 2025 19:10
@JiliDong-NOAA
Copy link
Contributor

@JiliDong-NOAA You mentioned previously that this work would change tests using saSAS, but shouldn't it also change results for tests using RUC LSM? That is what I'm seeing so far when I'm running RTs on Hera. Please take a look at the test_changes.list file and verify that the answer changes are expected (when it is pushed in an hour or so).

@grantfirl you are right. It will also change tests using both RUC LSM and inline post as the original RRFS production PR had the RUC LSM inline post fix combined. I will take a look at the log and the list.

@rhaesung
Copy link
Contributor

@jkbk2004 @grantfirl This PR prevents the cpld_restart_gfsv17 test from completing when compared to the new baseline similar to PR 2487 . Debugging is underway.

@grantfirl
Copy link
Collaborator Author

@jkbk2004 @grantfirl This PR prevents the cpld_restart_gfsv17 test from completing when compared to the new baseline similar to PR 2487 . Debugging is underway.

Thanks @rhaesung for helping to address this: ufs-community/ccpp-physics#252 (comment). I'm re-running pre-tests on Hera for good measure.

@jkbk2004
Copy link
Collaborator

@grantfirl go ahead to sync up branches. we will work on this pr.

@grantfirl
Copy link
Collaborator Author

@grantfirl go ahead to sync up branches. we will work on this pr.

OK, ready to go.

@jkbk2004 jkbk2004 added Baseline Updates Current baselines will be updated. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. jenkins-ort run ORT testing labels Feb 17, 2025
@jkbk2004 jkbk2004 removed the jenkins-ort run ORT testing label Feb 18, 2025
@FernandoAndrade-NOAA
Copy link
Collaborator

@jkbk2004 FYI looks like there's also a permission issue with the branch for Jenkins. @grantfirl can you add permissions for the epic-cicd-jenkins account?

@ligiabernardet
Copy link
Collaborator

@grantfirl is on leave this week. Is this something that someone else can address?

@FernandoAndrade-NOAA
Copy link
Collaborator

I'm not sure if anyone else would be able to edit the branch permissions, but I was able to grab the Jenkins logs from its directory.

@BrianCurtis-NOAA
Copy link
Collaborator

If it doesn't fail generating the logs, then as long as they're passed, just manually add them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants