-
Notifications
You must be signed in to change notification settings - Fork 146
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
NRL Neptune model 32-bit physics support #918
Conversation
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.
This looks familiar and OK to me, although it looks like it needs a merge with main given the conflict. The status of this is that it compiles in single precision and runs RTs successfully in double precision?
I did not apply the changes needed at the ufs-weather-model or fv3atm level to get it to compile with 32 bits. That would be easy to add. Should I? |
I don't know that we need to merge changes in parent repos in order to enable compiling, but I'm guessing that this code should be able to compile before we merge it (this was Dom's requirement). Has it complied successfully in single precision? |
You'd need pretty much everything from my other 32-bit PR. It would just be a non-draft version of that PR. Ligia's instructions were to do a PR that only compiles 64 bits, but contains all of the CCPP 32-bit changes. That is this PR. She said the next step is to get it to actually run, which is the purpose of the other PR. At this point, the other PR will compile 32 bit but not run, and this one won't compile or run 32 bits. |
If it can compile in 32 bit mode using changes in the other draft PR, then that is good enough for me. |
As of this posting, this PR is up-to-date with all submodules and has passed intel compiler tests (on jet) and gnu compiler tests (on hera). |
physics/machine.F
Outdated
&, kind_rad = 4 & | ||
&, kind_phys = 4 ,kind_taum=4 & | ||
&, kind_grid = 4 & | ||
&, kind_grid = 8 &! atmos_cubed_sphere requres kind_grid=8 | ||
&, kind_REAL = 4 &! used in cmp_comm | ||
&, kind_LOGICAL = 4 & | ||
&, kind_INTEGER = 4 ! -,,- |
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 would rewrite this big #ifndef / #else / #endif block as:
integer, parameter :: kind_io4 = 4, kind_io8 = 8 , kind_ior = 8 &
&, kind_evod = 8, kind_dbl_prec = 8 &
&, kind_sngl_prec = 4
# ifdef __PGI
&, kind_qdt_prec = 8 &
# else
&, kind_qdt_prec = 16 &
# endif
&, kind_LOGICAL = 4 &
&, kind_INTEGER = 4 ! -,,-
#ifdef SINGLE_PREC
integer, parameter :: kind_rad = kind_sngl_prec &
&, kind_phys = kind_sngl_prec &
&, kind_taum = kind_sngl_prec &
&, kind_grid = kind_dbl_prec &! atmos_cubed_sphere requres kind_grid=8
&, kind_REAL = kind_sngl_prec ! used in cmp_comm
#else
integer, parameter :: kind_rad = kind_dbl_prec &
&, kind_phys = kind_dbl_prec &
&, kind_taum = kind_dbl_prec &
&, kind_grid = kind_dbl_prec &! atmos_cubed_sphere requres kind_grid=8
&, kind_REAL = kind_dbl_prec ! used in cmp_comm
#endif
avoiding unnecessary (repeated) definition of kinds that are equal in both cases.
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.
Also, I do not see where kind_qdt_prec
is used. I think it can be removed if it's not used anywhere.
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 think kind_qdt_prec is used in the Neptune fork. I'd rather leave it since it's not causing any troubles now.
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 think kind_qdt_prec is used in the Neptune fork. I'd rather leave it since it's not causing any troubles now.
Neptune's fork of ccpp-physics? If they are using it in the code that's not in this repo it should be defined in their code, not here. I do not think this repo should 'collect' random definitions used elsewhere.
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.
What about kind_taum
?
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 see what you mean. Let's wait to hear from NRL, given what you just said you are probably correct that it isn't used and can be removed. machine.F
was ported from IPD as-is a long time ago and basically not updated since then.
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.
The machine.F is used outside ccpp, so it is possible the Neptune dynamical core is using kind_taum or kind_qdt_prec
If this is the case, than that's even bigger reason to have definitions of kinds ONLY used by the code in this repo, and no any other kind, since code that depends on ccpp physics should only use definitions defined here to interface their code with the ccpp physics, ie. to declare variables used in the ccpp-physics public API. This creates unnecessary dependency.
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.
@climbfuji @SamuelTrahanNOAA @DusanJovic-NOAA Thank you for including us in this discussion. At NRL, we only use kind_phys, kind_dbl_prec, and kind_sngl_prec from machine.F. @michalakes added the single precision kind and can comment more.
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.
The dbl and sngl types are necessary for the single precision option. But I gather from back-reading the thread that those are not at issue. With regard to kind_qdt_prec
, I don't know where that comes from. It's not used in the NEPTUNE version of the code, except where it's defined in machine.F . But as Sam suggested, it doesn't do any harm whatever it might be for. I don't have an opinion whether to keep it or not.
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've cleaned up that section of the code in a similar manner to what Dusan suggested. I've removed kind_taum and kind_qdt_prec since they don't seem to serve a purpose for UFS or NRL at the moment.
I'm hoping @michalakes can review this and comment on the types. He has no repository access yet though, so I'll have to add him and hope he answers soon. |
@SamuelTrahanNOAA @grantfirl UFS RT passed. Ready for merge. |
…d)+cherry-pick (5544dab - 42184e7) commit 77bcfb1 Merge: f13ed4e 2d2f1a6 Author: Grant Firl <[email protected]> Date: Fri Jun 3 09:35:42 2022 -0400 Merge pull request NCAR#903 from lisa-bengtsson/prog_closure Add prognostic cumulus closure description in saSAS commit 2d2f1a6 Merge: 48f4274 f13ed4e Author: Lisa Bengtsson <[email protected]> Date: Fri May 27 13:53:46 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit f13ed4e Merge: 6e58242 942f9ad Author: Grant Firl <[email protected]> Date: Thu May 26 16:34:40 2022 -0400 Merge pull request NCAR#918 from SamuelTrahanNOAA/ccpp-neptune NRL Neptune model 32-bit physics support commit 48f4274 Merge: fc79cc3 6e58242 Author: Lisa Bengtsson <[email protected]> Date: Thu May 26 15:05:43 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit 942f9ad Author: samuel.trahan <[email protected]> Date: Wed May 25 22:08:44 2022 +0000 correct bug in machine.F commit 641544c Merge: d4d0b71 6e58242 Author: samuel.trahan <[email protected]> Date: Wed May 25 18:40:09 2022 +0000 Merge remote-tracking branch 'community/main' into ccpp-neptune commit 6e58242 Merge: 01e3d6b 8dae03a Author: Grant Firl <[email protected]> Date: Wed May 25 13:54:59 2022 -0400 Merge pull request NCAR#924 from dustinswales/update_rte_for_CCPP_v6 Update rte-rrtmgp submodule + SCM-only bugfix commit d4d0b71 Author: samuel.trahan <[email protected]> Date: Tue May 24 18:18:45 2022 +0000 Simplify machine.F and remove unused types. commit 828f168 Merge: 63020ec 01e3d6b Author: samuel.trahan <[email protected]> Date: Mon May 23 18:50:11 2022 +0000 Merge NCAR main commit 8dae03a Merge: b994063 87359d2 Author: dustinswales <[email protected]> Date: Fri May 20 09:46:35 2022 -0600 Merge pull request NOAA-GSL#18 from grantfirl/fix_SCM_specified_surface_flux_bug Fix scm specified surface flux bug commit 87359d2 Merge: 49c7096 01e3d6b Author: Grant Firl <[email protected]> Date: Fri May 20 09:13:55 2022 -0400 Merge branch 'main' into fix_SCM_specified_surface_flux_bug commit fc79cc3 Author: Lisa Bengtsson <[email protected]> Date: Thu May 19 19:59:43 2022 +0000 Change intent to inout for conditional variables commit 96d0d36 Merge: 6f38cc6 01e3d6b Author: Lisa Bengtsson <[email protected]> Date: Thu May 19 00:14:56 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit 6f38cc6 Author: Lisa Bengtsson <[email protected]> Date: Wed May 18 23:58:09 2022 +0000 address some review comments, fix decomposition error, correct bug in initialization commit b994063 Author: Dustin Swales <[email protected]> Date: Mon May 16 09:16:30 2022 -0600 Update rte-rrtmgp submodule commit 49c7096 Author: Grant Firl <[email protected]> Date: Tue May 10 13:49:55 2022 -0400 make sure that tsfc_wat is calculated when wet = T commit 63020ec Author: samuel.trahan <[email protected]> Date: Thu May 5 22:46:10 2022 +0000 Switch to another version of the code that works with 64 bit commit 3dec4e6 Merge: be534e7 3405ff1 Author: Lisa Bengtsson <[email protected]> Date: Thu May 5 04:20:02 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit be534e7 Author: Lisa Bengtsson <[email protected]> Date: Thu May 5 04:08:06 2022 +0000 addressing some review comments commit e7c42c7 Author: samuel.trahan <[email protected]> Date: Thu May 5 00:43:21 2022 +0000 Move some code to modules commit de90593 Merge: 6871a93 3405ff1 Author: samuel.trahan <[email protected]> Date: Wed May 4 22:12:14 2022 +0000 Merge remote-tracking branch 'community/main' into sing_prec_from_main commit 6871a93 Author: Samuel Trahan <[email protected]> Date: Wed May 4 17:32:24 2022 +0000 Changes needed for 32-bit physics commit 527e1b9 Author: samuel.trahan <[email protected]> Date: Mon May 2 22:11:42 2022 +0000 Pass -DCCPP_SINGLE_PRECISION from cmake to -DSINGLE_PREC in cpp commit aff574b Merge: 239710b 7e35351 Author: samuel.trahan <[email protected]> Date: Mon May 2 16:02:18 2022 +0000 Merge remote-tracking branch 'community/main' into sing_prec_from_main commit 8b815e0 Author: Lisa Bengtsson <[email protected]> Date: Fri Apr 29 03:03:46 2022 +0000 address some bugs caught by debug flag commit e2d5a2a Author: Lisa Bengtsson <[email protected]> Date: Wed Apr 27 22:40:40 2022 +0000 cleaning out some print statements commit 0200e2d Author: Lisa Bengtsson <[email protected]> Date: Wed Apr 27 18:48:38 2022 +0000 addressing some review comments commit e969672 Merge: a9b439f 7e35351 Author: Lisa Bengtsson <[email protected]> Date: Fri Apr 22 21:10:40 2022 +0000 merge with upstream commit a9b439f Merge: fc7e7a0 860245c Author: Lisa Bengtsson <[email protected]> Date: Fri Apr 22 04:07:35 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit fc7e7a0 Author: Lisa Bengtsson <[email protected]> Date: Fri Apr 22 04:00:31 2022 +0000 addressing some review comments commit 89eaad9 Merge: b530db1 e2806f0 Author: Lisa Bengtsson <[email protected]> Date: Thu Apr 21 15:05:15 2022 +0000 Merge branch 'main' of https://github.com/NCAR/ccpp-physics into prog_closure commit b530db1 Author: Lisa Bengtsson <[email protected]> Date: Wed Apr 20 01:29:13 2022 +0000 cleaning some diagnostics commit 4f84ed7 Author: Lisa Bengtsson <[email protected]> Date: Tue Apr 19 19:22:53 2022 +0000 add shallow convection closure updates, add ntsigma in generic files commit 842eae3 Author: Lisa Bengtsson <[email protected]> Date: Mon Apr 18 15:05:40 2022 +0000 Ensuring the moisture budget is correct via PBL, microphysics coupling commit 235ec38 Author: Lisa Bengtsson <[email protected]> Date: Wed Apr 13 17:44:27 2022 +0000 add progsigma_calc commit 239710b Merge: b17a7e7 8500cea Author: Grant Firl <[email protected]> Date: Thu Jan 13 11:06:52 2022 -0700 Merge branch 'main' into sing_prec_from_main commit b17a7e7 Merge: 805c62c 623feaa Author: Grant Firl <[email protected]> Date: Tue Dec 7 11:00:17 2021 -0700 Merge branch 'main' into sing_prec_from_main commit 805c62c Author: Grant Firl <[email protected]> Date: Tue Dec 7 10:39:05 2021 -0700 add single precision code changes from michalakes fork, jm-nrl-32bitfp-24cc09e branch
This adds changes from NRL to add 32-bit physics support to their model. The changes are a subset of #772 and it replaces the prior attempt at this PR, #797