-
Notifications
You must be signed in to change notification settings - Fork 458
Shared c++/Fortran constants other than Physical Constants Dictionary #8082
Replies: 7 comments · 19 replies
-
|
EAMxx also has a bunch of constants that should be reorganized better. It's been a "low" priority for ages, so nobody ever tried to take a stab at a better framework. But now we have bots, so if we come up with some good idea, we can throw copilot at it. @xylar can you elaborate what you have in mind? Are you suggesting components migrate their constants to pcd? Or that components have their own (well documented) I do favor moving more and more constants to pcd, as long as they are "shareable". It doesn't make much sense (I think) to put component specific constants into pcd. However, it is hard to know whether another component also uses a constant without a general, easy-to-query, database. Perhaps we should have the pcd repo also store component-level yaml files (like It is a bit clunky as a workflow, especially as the yaml files grow big and we have to search more. I'd love to hear a more robust (and maybe easier) approach... Perhaps we can delegate to bot the checks. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I agree constants specific to a physical parameterization belong just in that code or a file in the same directory. The criteria should be "Might 2 or more top level components (ocean, atm, sea ice) need to know this?" |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Right, but there are |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
@bartgol, I like your proposal. The issue for us is that Omega and MPAS-Ocean should share various constants, and Polaris should also be using the same values for these. So something like I also agree that there's a certain amount of negotiation that has to happen to determine which constants are shared and which are component-specific. But there are quite a few that are pretty obvious -- reference densities, heat capacities, etc. are fairly obviously involved in the fluxes between components. |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
If we were following some kind of classic development model where a scientist writes the spec and developer implements, the spec would say "get this constant from PCD". A developer shouldn't have to guess or go searching. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
tagging @proteanplanet |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Also tagging @rmontuoro |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Looking at https://github.com/E3SM-Project/Omega/blob/develop/components/omega/src/ocn/GlobalConstants.h, I'd say everything under |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Looking at https://github.com/E3SM-Project/E3SM/blob/master/share/util/shr_const_mod.F90, everything there should also be in PCD except the SPVAL ones. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I don't see any evidence that I think |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I opened E3SM-Project/PhysicalConstantsDictionary#17 to hopefully address many parts of this discussion item. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I'm not sure this is quite the right place to bring this up, but it perhaps should be stated: A significant problem we need to address that the Physical Constants Dictionary does not alone address is in lookup tables, of which the poster child is the RRTMG longwave scheme, which uses literal constants here: E3SM/components/eam/src/physics/rrtmg/ext/rrtmg_lw/rrtmg_lw_init.f90 Lines 236 to 244 in c1ed69e
which @lilimanzo thinks were likely used to generate these lookup tables: E3SM/components/eam/src/physics/rrtmg/ext/rrtmg_lw/rrtmg_lw_setcoef.f90 Lines 564 to 1267 in c1ed69e
There are lookup tables for radiation in other parts of the model (land, sea ice) that we will likely need to similarly align with the Physical Constants Dictionary. This is perhaps the elephant in the room, aside from tracking down all the literal constants throughout E3SM. (tagging @crterai and @susburrows) |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
@vanroekel, MPAS-Ocean already has some documented inconsistencies with the rest of the model (e.g. #6387). Updating |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Thanks for reminding me of that @xylar. I don't have a good sense of the effort needed to update MPAS-Ocean to be consistent, but given the pervasiveness of things like |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
The key point of my post is not about which model or code is being updated. It's that the longwave atmospheric physics, and other radiation physics in E3SM, including in MPAS-SeaIce and ELM, is dependent on lookup tables that must be generated using precisely the same constants as specified in the global dictionary, otherwise our constants are not unified and we can have a significant energy leak. For all the code that forms the backbone of V4, we need to make sure all of our lookup tables comply with the dictionary. |
Beta Was this translation helpful? Give feedback.
All reactions
-
I very much agree with this, and we'll make sure Omega is consistent with the dictionary. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
We can work towards having the constants in our look up tables comply with the dictionary for the v4 release as well. |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
On the other hand, I kind of want to make v3 consistent just to see what happens. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Good point. @lilimanzo is preparing a paper on the longwave leak, and we'll keep you appraised of that. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I'm not comfortable devoting a lot of effort from Omega to fixing MPAS-Ocean constants with the tight timelines for omega delivery |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
As far as the radiation leak goes, MPAS-Ocean is off the hook. There is, however, work currently in review for GRL that switches the ocean to a grey body that is relevant to Omega, and could be considered for V4.1. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Happy to consider that change for v4.x for omega. I just don't want to spend effort from Omega on v3 changes |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
We currently have a good process going for taking the yaml file from the Physical Constants Dictionary and turning it into C++ and Fortran code to be imported into E3SM components.
My view is that there is a need for a similar set of model-specific constants, equivalent to those currently in shr_const_mod.F90 but that would similarly be available in yaml, C++ and Fortran formats, not just Fortran.
Omega is currently hard-coding its own set of constants in its GlobalConstants.h, but we are fully aware that this is not a desirably final solution. Instead, we would like to agree, along with other components, on a way to migrate
shr_const_mod.F90to a workflow that is similar to PCD.Beta Was this translation helpful? Give feedback.
All reactions