search

NCAR / ccpp-physics

GFS physics for CCPP
Other
56 stars 145 forks source link

Uniformly apply REAL(kind=kind_phys) from CCPP-Physics machine.F definitions to MYNN Surface scheme and wrapper #985

Closed timsliwinski-noaa closed 1 year ago

timsliwinski-noaa commented 1 year ago

Uniformly apply REAL(kind=kind_phys) from CCPP-Physics machine.F definitions to MYNN Surface scheme and wrapper

Description of changes: This change modifies REAL-type variables and specific REAL-type constants so that they uniformly utilize the kind=kind_phys definition for REAL types found in the CCPP-physics machine.F file. This reduces casting and recasting of REALs between types of differing precision. Additional changes include changing occurances of ALOG to LOG to support more than single precision REALs as needed when kind=kind_phys is defined in that manner.

Testing: Testing was performed using CCPP-SCM. Testing included standalone compilation as well as compilation as part of the CCPP-SCM framework. Runtime testing was performed and output from a TWPICE RRFS v1 beta case utilizing the MYNN Surface Scheme was compared between runs performed before and after changes in this commit. Aside from expected changes in rounding errors and the accumulation thereof, significant changes were not observed in the output.

timsliwinski-noaa commented 1 year ago

Thank you, Joseph. Happy to help. I'm also working to port this scheme over to GPUs using OpenACC so this was precursor work to that. More to come on the GPU front in the future, but I'll watch for your updates.

dustinswales commented 1 year ago

@timsliwinski-noaa @joeolson42 If this is intended for UFS applications, which it appears to be, can you reopen this PR into the ufs-community:ccpp-physics repository?

timsliwinski-noaa commented 1 year ago

@dustinswales I didn't specifically have UFS in mind when I was creating this particular PR, though I could see it having importance there as well. I mainly saw it as more of a general CCPP infrastructure improvement since CCPP has machine.F to specify precision, but the MYNN surface scheme didn't use it. However, if you think it fits better over there, I'll gladly move this PR over to the repo you mentioned. This is my first contribution to CCPP, so it's possible I'm in the wrong place.

dustinswales commented 1 year ago

@timsliwinski-noaa Actually, since you started from the SCM it makes sense that you would open the PR here. Sorry for any confusion.

dustinswales commented 1 year ago

@timsliwinski-noaa @joeolson42 I tested this change in the UFS and the tests with do_mynnedmf=T fail when comparing to the baseline. Test 045 regional_control FAIL Test 047 regional_decomp FAIL Test 048 regional_2threads FAIL Test 049 regional_noquilt FAIL Test 050 regional_netcdf_parallel FAIL Test 051 regional_2dwrtdecomp FAIL Test 052 regional_wofs FAIL Test 053 rap_control FAIL Test 054 rap_rrtmgp FAIL Test 055 regional_spp_sppt_shum_skeb FAIL Test 056 rap_decomp FAIL Test 057 rap_2threads FAIL Test 062 hrrr_control FAIL Test 063 hrrr_control_decomp FAIL Test 064 hrrr_control_2threads FAIL Test 066 rrfs_v1beta FAIL Test 067 rrfs_v1nssl FAIL Test 068 rrfs_v1nssl_nohailnoccn FAIL Test 069 rrfs_conus13km_hrrr_warm FAIL Test 070 rrfs_smoke_conus13km_hrrr_warm FAIL Test 071 rrfs_conus13km_radar_tten_warm FAIL Test 072 rrfs_conus13km_hrrr_warm_2threads FAIL Test 073 rrfs_conus13km_radar_tten_warm_2threads FAIL Test 078 rrfs_conus13km_hrrr_warm_debug FAIL Test 079 rrfs_conus13km_radar_tten_warm_debug FAIL Test 091 regional_debug FAIL Test 102 rap_rrtmgp_debug FAIL Test 107 regional_spp_sppt_shum_skeb_dyn32_phy32 FAIL Test 108 rap_control_dyn32_phy32 FAIL Test 109 hrrr_control_dyn32_phy32 FAIL Test 110 rap_2threads_dyn32_phy32 FAIL Test 111 hrrr_control_2threads_dyn32_phy32 FAIL Test 112 hrrr_control_decomp_dyn32_phy32 FAIL Test 115 rap_control_dyn64_phy32 FAIL

In the error files there are the following statements: 138: ITIMESTEP= 1 iter= 1 138: === important input to mynnsfclayer, i: 1 138: wet= T pblh= 0.000000000000000E+000 tsk= 288.081717227938 tsurf= 138: 288.081717227938 qsfc= 1.040756341781887E-002 znt= 138: 1.773057456732028E-007 ust= 8.882021587236011E-002 snowh= 138: 0.000000000000000E+000 psfcpa= 102034.557907525 dz= 138: 20.9147473579392 qflx= 0.000000000000000E+000 hflx= 138: 0.000000000000000E+000 hpbl= 0.000000000000000E+000 138: === important input to mynnsfclayer, i: 2 138: wet= T pblh= 0.000000000000000E+000 tsk= 286.417232792059 tsurf= 138: 286.417232792059 qsfc= 9.352438638789169E-003 znt= 138: 1.000000000000000E-007 ust= 6.261110126383368E-002 snowh= 138: 0.000000000000000E+000 psfcpa= 101864.437421870 dz= 138: 20.7603693313538 qflx= 0.000000000000000E+000 hflx= 138: 0.000000000000000E+000 hpbl= 0.000000000000000E+000

Any ideas?

timsliwinski-noaa commented 1 year ago

@dustinswales From your output, it appears I may have missed turning off the debug option in the MYNN surface scheme module file before I created the pull request. Can you change the debug option to 0 rather than 1 on line 115 of module_sf_mynn.F90 and let me know if that clears everything up? If so, I'll push that change to the branch. If that doesn't fix it, I'll have to dig further to see what else may be wrong.

dustinswales commented 1 year ago

@timsliwinski-noaa The RTs are successful with Intel on NCARs Cheyenne with the debugging option change you suggested. After discussing this PR with the other code-managers (@grantfirl @ChunxiZhang-NOAA), it makes sense for this PR to be moved to the UFS fork of the ccpp-physics. I opened a PR in the UFS ccpp-physics fork, with this small change added on top . Here are the corresponding links to the PRs needed by the UFS supermodules: FV3, UWM.