Skip to content

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

Closed
timsliwinski-noaa wants to merge 1 commit into
NCAR:mainfrom
timsliwinski-noaa:mynnsfc_uniform_real_kind
Closed

Uniformly apply REAL(kind=kind_phys) from CCPP-Physics machine.F definitions to MYNN Surface scheme and wrapper#985
timsliwinski-noaa wants to merge 1 commit into
NCAR:mainfrom
timsliwinski-noaa:mynnsfc_uniform_real_kind

Conversation

@timsliwinski-noaa
Copy link
Copy Markdown
Contributor

@timsliwinski-noaa timsliwinski-noaa commented Nov 17, 2022

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.

…nitions where necessary to improve consistency in precision

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.
@timsliwinski-noaa timsliwinski-noaa changed the title Uniformly apply REAL(kind=kind_phys) from CCPP-Physics machine.F definitions Uniformly apply REAL(kind=kind_phys) from CCPP-Physics machine.F definitions to MYNN Surface scheme and wrapper Nov 17, 2022
Copy link
Copy Markdown
Collaborator

@joeolson42 joeolson42 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I do have an update coming for this scheme and I had added the kind_phys declarations but not as thoroughly as you did it (even in the max/mins). Also, thanks for removing the trailing spaces. I am skeptical they are coming from my editor...

@timsliwinski-noaa
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Member

@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
Copy link
Copy Markdown
Contributor Author

timsliwinski-noaa commented Nov 17, 2022

@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
Copy link
Copy Markdown
Member

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

@dustinswales
Copy link
Copy Markdown
Member

@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
Copy link
Copy Markdown
Contributor Author

timsliwinski-noaa commented Dec 2, 2022

@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
Copy link
Copy Markdown
Member

dustinswales commented Dec 6, 2022

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants