Logic error in "dtend" diagnostic outputs for "generic_tendency" schemes

[mdtoy]

Description

In the Unified GWD schemes (unified_ugwp.F90 and ugwpv1_gsldrag.F90), the tendency outputs ("dtend") are flagged by the logical variables ldiag3d, lssav and flag_for_gwd_generic_tend for both the "orographic_gwd" and "nonorographic_gwd" processes (see example code snippet below). The condition for outputting the tendencies will never be met because "flag_for_gwd_generic_tend" will always be ".true." for the ugwp schemes. It is set to .true. in GFS_typedefs.F90 for gwd_opt.ne.1.

The fix to this problem is probably just to delete the ".not.flag_for_gwd_generic_tend" requirement in the if-statement.

I believe other physics schemes may have the same problem, so please look in to this. These are the schemes that involve the following flags:
flag_for_pbl_generic_tend
flag_for_dcnv_generic_tend
flag_for_scnv_generic_tend

Code snippet from unified_ugwp.F90:

  if(ldiag3d .and. lssav .and. .not. flag_for_gwd_generic_tend) then
    idtend = dtidx(index_of_x_wind,index_of_process_nonorographic_gwd)
    if(idtend>=1) then
      dtend(:,:,idtend) = dtend(:,:,idtend) + Pdudt*dtp
    endif

    idtend = dtidx(index_of_y_wind,index_of_process_nonorographic_gwd)
    if(idtend>=1) then
      dtend(:,:,idtend) = dtend(:,:,idtend) + Pdvdt*dtp
    endif

    idtend = dtidx(index_of_temperature,index_of_process_nonorographic_gwd)
    if(idtend>=1) then
      dtend(:,:,idtend) = dtend(:,:,idtend) + Pdtdt*dtp
    endif
  endif

[climbfuji]

@mdtoy Can you fix this and issue #740 when you work on this schemes next time, please?

@SamuelTrahanNOAA FYI - we should probably check other schemes, too, although I think that the logic is correct in those places (at least nobody alerted us of zero tendencies from PBL or deep/shallow convection.

[SamuelTrahanNOAA]

This sounds like a misunderstanding of the implementation, not a bug.

Some schemes implement their own diagnostic tendencies, and some use the interstitials to determine tendencies. The flag_for_generic_whatever switches which code path is used. This has left some extra code in places where the tendencies could be calculated two different ways. The GFS_typedefs.F90 selects which calculation to use. If you remove those flags, you'll double-count tendencies.

I didn't see any problems in the dtend gravity wave drag tendencies in the suites I tested for my PR. Has that changed? Are the GWD tendencies zero?

[SamuelTrahanNOAA]

Correction: I never tested this scheme, so I don't know if this one works.

[SamuelTrahanNOAA]

Looking through the code, GFS_GWD_generic.F90 handles the tendencies. That code does work, and is well-tested. Look for this in that file:

if (lssav .and. ldiag3d .and. flag_for_gwd_generic_tend) then

and you will see the code that actually calculates the GWD tendencies.

[mdtoy]

Will do. — Mike

…

On Sep 24, 2021, at 2:44 PM, Dom Heinzeller ***@***.*** ***@***.***>> wrote: @mdtoy <https://github.com/mdtoy> Can you fix this and issue #740 <#740> when you work on this schemes next time, please? @SamuelTrahanNOAA <https://github.com/SamuelTrahanNOAA> FYI - we should probably check other schemes, too, although I think that the logic is correct in those places (at least nobody alerted us of zero tendencies from PBL or deep/shallow convection. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#741 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMODLCIPLAWEAURXN74M3TUDTPLTANCNFSM5EUGQS6Q>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[mdtoy]

Looking through the code, GFS_GWD_generic.F90 handles the tendencies. That code does work, and is well-tested. Look for this in that file:

if (lssav .and. ldiag3d .and. flag_for_gwd_generic_tend) then

and you will see the code that actually calculates the GWD tendencies.

@SamuelTrahanNOAA I agree that the tendencies will be handled in GFS_GWD_generic.F90 for flag_for_gwd_generic_tend = .true. My point, however, is that if unified_ugwp.F90 and ugwpv1_gsldrag.F90 are called, the user should have selected gwd_opt = 2, which would make flag_for_gwd_generic_tend set equal to .true. in GFS_typedefs.F90, so the dtend chunk of code will never be called because .not.flag_for_gwd_generic_tend will always be .false. So we may as well delete that chunk of code. However, note that in "unified_ugwp" and "ugwpv1_gsldrag", the "dtends" being output are for "non_orographic" processes (which are important, so we should call the code by eliminating the ".not.flag_for_gwd_generic_tend" flag so it does get called for the generic GWD schemes. The tendencies output in "GFS_GWD_generic" are "orographic", so there is no redundancy.
I hope this explanation makes sense.

[dustinswales]

@mdtoy I'm making an effort to address as many issues as possible. It seems that this has not been resolved yet?

My understanding of the bug is that the convective GWD tendency is not being output correctly, for a couple of reasons.

• In the unified scheme, there is a logical error. Outputting the tendency is conditioned on flag_for_gwd_generic_tend, which it should not be.
• In the gsldrag scheme, there is an indexing error 740.

Here are proposed fixes: dustinswales@15503ee

I also noticed another issue that I would like your feedback on. When I fixed the logical error in the unified scheme to address issue 741, I noticed that the convective gwd tendencies being output were actually the convective+orographic tendencies. dustinswales@3cf8895

If you agree with these changes I can include them in an upcoming cleanup PR.

[SamuelTrahanNOAA]

Is there any situation in which flag_for_gwd_generic_tend should be set? If not, it would be better to remove that flag from the code entirely.

Also, consider adding a regression test for one of the schemes with a broken dtend calculation. Pick one of the C96 configurations that uses the scheme and add a "dtend" test for it.

[dustinswales]

@SamuelTrahanNOAA I agree that flag_for_gwd_generic_tend is probably not needed, and some more cleanup too.

Actually, I think it would be cleaner for both (orographic and convective) diagnostic tendencies for the GWD schemes to be written in GFS_GWD_generic_post.F90, and removed from cires_ugwp.F90, ugwpv1_gsldrag.F90, and unified_ugwp.F90?
The same thing is done in four different routines, and I think this may have led to inconsistencies among them...

For example, in ugwpv1_gsldrag, the tendencies (dudt, dvdt, dtdt) are accumulated (input + oro + conv). Then in unifed_ugwp they are accumulated, but not the convective part (input + oro). And then in cires_ugwp they pass through the scheme untouched (input). In all three cases, these are the tendencies that are stored as the diagnostic orographic GWD tendency in GFS_GWD_generic_post_run().

Since it's not the diagnostic tendencies themselves that are inconsistent, but rather the interstitial fields (standard_names: process_split_cumulative_tendency_****) used to compute the tendencies are different, can this have prognostic implications?