-
Notifications
You must be signed in to change notification settings - Fork 147
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
GPU-enabled version of Grell-Freitas convection #859
Conversation
@@ -7,7 +7,7 @@ module cu_gf_driver | |||
! DH* TODO: replace constants with arguments to cu_gf_driver_run | |||
!use physcons , g => con_g, cp => con_cp, xlv => con_hvap, r_v => con_rv | |||
use machine , only: kind_phys | |||
use cu_gf_deep, only: cu_gf_deep_run,neg_check,fct1d3 | |||
use cu_gf_deep, only: cu_gf_deep_run,neg_check,autoconv,aeroevap,fct1d3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note. Importing symbols that aren't used does not make sense. However, it's one of the many bugs in the NVIDIA/PGI compilers that are used for the GPU testing - they often request symbols to be imported even when they don't get used. One of the reasons why we dropped PGI support for the UFS in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would we implement a regression test for this? Do the RDHPCS or WCOSS2 clusters have GPU nodes that can support this?
Not foreseen (with the UFS). The testing with the UFS will be for the CPU version (same file, just ignoring the acc directives). The Single Column Model (SCM) will be able to test the GPU directives with a few upcoming changes in the ccpp-scm repository. |
Is this intended to be able to run in the UFS? |
As far as I know (but little do I know these days), there is no plan to run physics on GPUs in th near term future in the UFS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no expertise in acc directives, so I cannot review those changes, but based on the regression test results and the non-acc directive changes, this should be fine to merge. Thanks @danielabdi-noaa and @climbfuji
This PR augments the Grell-Freitas convection schemes to be runnable on GPUs.
Mostly this is in form of acc directives, but a few minor code modifications were made to avoid inefficient operations on GPUs. For example, replace a string identifier with an integer switch.
These new schemes were regression tested on Hera with Intel and GNU. The results are summarized here: ufs-community/ufs-weather-model#1043
All credit for this work goes to @danielabdi-noaa (NOAA-GSL).
Fixes #860
Dependent PRs:
NOAA-EMC/fv3atm#479
ufs-community/ufs-weather-model#1043