MYNN sfclay restart reproducibility

[climbfuji]

This PR contains bug fixes for MYNN sfclay to get bit-for-bit identical results in restart runs.

Associated PRs:

#822
NOAA-EMC/fv3atm#455
ufs-community/ufs-weather-model#984

For regression testing, see ufs-community/ufs-weather-model#984

[tanyasmirnova]

@climbfuji The changes look good to me. Thank you for fixing the restart problem.

[joeolson42]

I'm surprised removing this helps. I don't see u* initialized before it's first used.

[climbfuji]

ust_{wat,ice,lnd} are initialized in GFS_surface_composites_pre_run, see lines 206-242.

[joeolson42]

I see... If this helps with restarts, then OK, but I don't know how uustar is being initialized in a cold start, so I may add this code back into the wrapper eventually and make sure it's not used for restarts. Does that seem reasonable?

[climbfuji]

According to FV3/io/FV3GFS_io.F90, it comes from the initial conditions. It's in the INPUT/sfc_data.tile?.nc files, for example:

data:

 uustar =
  0.171213456605573, 0.245438690554831, 0.256640811047583, 0.267651122786174,
    0.280224842133838, 0.279971796758598, 0.298066313514827,
    0.314749952182947, 0.33006493572479, 0.343210265083255,
    0.351366633436041, 0.362353378760175, 0.369426004804981,
    0.382825370910827, 0.40201134915115, 0.416131400590514,
    0.443041998906594, 0.453063344833092, 0.451435060306367,
    0.452779314754435, 0.432702751758146, 0.411955027777123,
    0.370880666443607, 0.310652664808393, 0.265327782215652, 0.2247260561156,
    0.263029915756367, 0.266947287025747, 0.296322723516394,
    0.308662654500799, 0.288600599239451, 0.180472574147917,
    0.163883864828559, 0.125231393380142, 0.154961388994689,
    0.146057388032206, 0.136944259693473, 0.237380508574376,
    0.305121627034509, 0.517771283751996, 0.568027172739538,
    0.50736289058438, 0.482317351376341, 0.413385954256456,
    0.400374229553073, 0.370092052409071, 0.336648494634005,
    0.300564612619546, 0.283338773802275, 0.341238177586638,
    0.261152207285838, 0.224116424328721, 0.215924232511202,
    0.189917101300733, 0.199322285712027, 0.157243532127691,
    0.19359710752422, 0.187451240086616, 0.196309269556824,
    0.198743901054553, 0.215083947166383, 0.272523138480303,
    0.274216651200398, 0.29467402005329, 0.276077397930394,
    0.228904980612239, 0.234033125425608, 0.258624364207327,
    0.311573093934831, 0.303495080141041, 0.25989005987661,
    0.218364635421538, 0.207690646375217, 0.212594973673417,
    0.215970389337089, 0.219594971489457, 0.21399290992621,
    0.213915985996377, 0.192268435554861, 0.174167760364215,
    0.140072079656223, 0.138048994889202, 0.0790837518180875,
    0.0403957897147722, 0.0827031933453741, 0.0864850134003294,
    0.170175184707559, 0.23613873730674, 0.146922292088942,
    0.211363181392024, 0.243766474618606, 0.296856229449771,
    0.397668207915114, 0.450776965678785, 0.508171170652395, 0.523651537042418,
  0.0820358681750062, 0.157450881891468, 0.277328270778228,

We rely on the model initializing certain surface properties for many variables. If we don't want to do this anymore, then we are opening a can of worms because we will have to check for hundreds of variables. The compiler flags ensure that uustar cannot be uninitialized (but they can't make sure that the variable isn't set to zero).

[joeolson42]

I think initializing u* from a previous run (especially if the IC is from a different model suite) is questionable. It makes perfect sense for a restart but for cycling mode, the background can be changed enough by DA to warrant a fresh u* calculation to be consistent with the updated u, v, and near-surface stability. That said, it may not be a killer, since it only impacts the first timestep. Also, the way I was initializing u* is only an approximation anyway, so it may not be much/any better. Maybe for now, we keep this in mind and be ready to make a change. This is another issue that can make universalizing this scheme for WRF and CCPP more difficult because u* is expected to be initialized by the sfc layer schemes in WRF, but I suppose this could be handled in a wrapper.

…

On Wed, Dec 29, 2021 at 12:37 PM Dom Heinzeller ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/module_sf_mynn.F90 <#822 (comment)>: > ENDDO IF (itimestep==1 .AND. iter==1) THEN DO i=its,ite !Everything here is used before calculated - UST_WAT(i)=MAX(0.04*SQRT(U1D(i)*U1D(i) + V1D(i)*V1D(i)),0.001) - UST_LND(i)=MAX(0.04*SQRT(U1D(i)*U1D(i) + V1D(i)*V1D(i)),0.001) - UST_ICE(i)=MAX(0.04*SQRT(U1D(i)*U1D(i) + V1D(i)*V1D(i)),0.001) MOL(i)=0.0 According to FV3/io/FV3GFS_io.F90, it comes from the initial conditions. It's in the INPUT/sfc_data.tile?.nc files, for example: data: uustar = 0.171213456605573, 0.245438690554831, 0.256640811047583, 0.267651122786174, 0.280224842133838, 0.279971796758598, 0.298066313514827, 0.314749952182947, 0.33006493572479, 0.343210265083255, 0.351366633436041, 0.362353378760175, 0.369426004804981, 0.382825370910827, 0.40201134915115, 0.416131400590514, 0.443041998906594, 0.453063344833092, 0.451435060306367, 0.452779314754435, 0.432702751758146, 0.411955027777123, 0.370880666443607, 0.310652664808393, 0.265327782215652, 0.2247260561156, 0.263029915756367, 0.266947287025747, 0.296322723516394, 0.308662654500799, 0.288600599239451, 0.180472574147917, 0.163883864828559, 0.125231393380142, 0.154961388994689, 0.146057388032206, 0.136944259693473, 0.237380508574376, 0.305121627034509, 0.517771283751996, 0.568027172739538, 0.50736289058438, 0.482317351376341, 0.413385954256456, 0.400374229553073, 0.370092052409071, 0.336648494634005, 0.300564612619546, 0.283338773802275, 0.341238177586638, 0.261152207285838, 0.224116424328721, 0.215924232511202, 0.189917101300733, 0.199322285712027, 0.157243532127691, 0.19359710752422, 0.187451240086616, 0.196309269556824, 0.198743901054553, 0.215083947166383, 0.272523138480303, 0.274216651200398, 0.29467402005329, 0.276077397930394, 0.228904980612239, 0.234033125425608, 0.258624364207327, 0.311573093934831, 0.303495080141041, 0.25989005987661, 0.218364635421538, 0.207690646375217, 0.212594973673417, 0.215970389337089, 0.219594971489457, 0.21399290992621, 0.213915985996377, 0.192268435554861, 0.174167760364215, 0.140072079656223, 0.138048994889202, 0.0790837518180875, 0.0403957897147722, 0.0827031933453741, 0.0864850134003294, 0.170175184707559, 0.23613873730674, 0.146922292088942, 0.211363181392024, 0.243766474618606, 0.296856229449771, 0.397668207915114, 0.450776965678785, 0.508171170652395, 0.523651537042418, 0.0820358681750062, 0.157450881891468, 0.277328270778228, We rely on the model initializing certain surface properties for many variables. If we don't want to do this anymore, then we are opening a can of worms because we will have to check for hundreds of variables. The compiler flags ensure that uustar cannot be uninitialized (but they can't make sure that the variable isn't set to zero). If we question the initialization of uustar by the model, then we are open — Reply to this email directly, view it on GitHub <#822 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADLRR3UGT6TCGWTTOU57BKLUTNPQNANCNFSM5K6H5SXQ> . 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>. You are receiving this because your review was requested.Message ID: ***@***.***>

-- Joseph Olson Model Physics Branch Chief Environmental Prediction Advancement Division NOAA-Global Systems Laboratory Boulder, Colorado

[grantfirl]

Thanks for fixing this issue @climbfuji. These code changes look good to me from a CCPP point of view. @joeolson42 If you think that how some surface-related variables are initialized outside of restarts needs to be addressed, this should probably be added as a GitHub issue so that it isn't forgotten.

[hannahcbarnes]

Thanks for everyone's work on this issue.

[joeolson42]

@grantfirl I'm not too concerned. I can think of some cases like stable layer mix-outs, where u* from the previous forecast could be very inaccurate, nut in general it might be OK. I think we should keep this in mind but I think its OK to check it in as is.

[climbfuji]

@grantfirl I'm not too concerned. I can think of some cases like stable layer mix-outs, where u* from the previous forecast could be very inaccurate, nut in general it might be OK. I think we should keep this in mind but I think its OK to check it in as is.

@joeolson42 Would you be able to approve the PR? We are working on it today. Thanks!

[climbfuji]

@joeolson42 turns out I had to put back the ustar initialization, otherwise the rap_noah_debug test would crash. Thanks for pointing that out during your review, this way I found the culprit within minutes instead of hours.

[grantfirl]

@climbfuji Do you know if putting back those lines will affect the restart reproducibility that this PR is supposed to address?

[climbfuji]

No, it won't affect it.

…

On Jan 6, 2022, at 9:10 AM, Grant Firl ***@***.***> wrote: @climbfuji <https://github.com/climbfuji> Do you know if putting back those lines will affect the restart reproducibility that this PR is supposed to address? — Reply to this email directly, view it on GitHub <#822 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RPWKXZ5VF5VI7R2NIDUUW5GHANCNFSM5K6H5SXQ>. 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>. You are receiving this because you were mentioned.

[joeolson42]

Sorry, I've been powerpointing all day... Thanks for going ahead without me. You made good choices. -joe On Thu, Jan 6, 2022 at 9:11 AM Dom Heinzeller ***@***.***> wrote:

…

No, it won't affect it. > On Jan 6, 2022, at 9:10 AM, Grant Firl ***@***.***> wrote: > > > @climbfuji <https://github.com/climbfuji> Do you know if putting back those lines will affect the restart reproducibility that this PR is supposed to address? > > — > Reply to this email directly, view it on GitHub < #822 (comment)>, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AB5C2RPWKXZ5VF5VI7R2NIDUUW5GHANCNFSM5K6H5SXQ >. > 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>. > You are receiving this because you were mentioned. > — Reply to this email directly, view it on GitHub <#822 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADLRR3VHE7IPXKOXNG6E7ZLUUW5LRANCNFSM5K6H5SXQ> . 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>. You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Joseph Olson Model Physics Branch Chief Environmental Prediction Advancement Division NOAA-Global Systems Laboratory Boulder, Colorado