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 NSSL microphysics to fix ORT restart and decomposition tests #904

Merged
merged 8 commits into from
May 11, 2022

Conversation

MicroTed
Copy link
Contributor

@MicroTed MicroTed commented Apr 22, 2022

In the process of adding a new suite, it was found that rrfs_v1nssl did not pass the ORT restart (rst) and decomposition (dcp) tests. Both of these failures were tracked to mp_nssl.F90. The restart issue simply needed extra logic (using the 'restart' flag) to control the running of a subroutine on the first time step. The dcp test seems to be solved by replacing 0.0 with 0.0_rkind_phys.

ORT tests run on hera
RT for rrfs_v1nssl run on jet

Fixes issue raised in NOAA-EMC/fv3atm#514

@@ -2884,7 +2884,7 @@ SUBROUTINE nssl_2mom_driver(qv, qc, qr, qi, qs, qh, qhl, ccw, crw, cci, csw, chw
IF ( lccn > 1 .and. is_aerosol_aware .and. flag_qnwfa ) THEN
! not used here
ELSEIF ( present( cn ) .and. lccn > 1 .and. .not. flag_qndrop) THEN
IF ( lccna > 1 .and. .not. present( cna ) ) THEN
IF ( lccna > 1 .and. .not. ( present( cna ) .and. f_cnatmp ) ) THEN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this change is not directly related to the issue, but it was a logic error found along the way (no effect, because lccna=0 for current uses in CCPP)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great to know that you fixed this logic error.

@MicroTed MicroTed marked this pull request as ready for review April 22, 2022 16:05
Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

These changes all look fine to me. @junwang-noaa Should we point NOAA-EMC/fv3atm#514 to this branch for UFS RTs?

@junwang-noaa
Copy link
Collaborator

@grantfirl Yes, please point to fv3atm PR#504 so that we can get the reproducibility issue fixed in Ted's ufs PR.
@MicroTed Nice work! Thanks for fixing the issues!

@grantfirl
Copy link
Collaborator

@MicroTed We just merged in the previous ccpp-physics PR and we'll get yours in next. Please merge in the latest main branch of ccpp-physics (commit: 7e35351) into this branch before pointing your fv3atm PR branch to this.

@grantfirl
Copy link
Collaborator

Fixes #916

@grantfirl grantfirl merged commit 99f32c5 into NCAR:main May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CCPP v6 Needed for CCPP v6 public release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants