Skip to content
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

Noah MP must use host model constants instead of its own ones #582

Open
climbfuji opened this issue Mar 1, 2021 · 11 comments
Open

Noah MP must use host model constants instead of its own ones #582

climbfuji opened this issue Mar 1, 2021 · 11 comments
Assignees

Comments

@climbfuji
Copy link
Collaborator

Noah MP in several places uses its own constants, e.g. for the gravitational acceleration. This needs to be fixed to receive/set constants via the argument list. See #575.

@barlage
Copy link
Collaborator

barlage commented Mar 1, 2021

Is there an example, say in rrtmgp (since it is a separate repo), where this is done? Just browsing the land models, all of them define their own constants. I'm not saying this is the proper thing to do, but it seems we need a generalized approach to make this happen.

@grantfirl
Copy link
Collaborator

I wrote this in #575, but it belongs here:

"To be clear, I'm talking about a subset of the physical constants defined here. In particular, grav, sb, vkc, tfrz, hsub, hvap, hfus, cwat, cice, cpair, rair, rw, and denh2o are all defined by existing CCPP host models. And by "going through the CCPP", I'm referring to physics using physical constants defined by the host model so that all physics using them can use a consistent set. Otherwise, if every scheme has its own value of gravity, specific heats, etc., this introduces a lot of inconsistencies. If other users of the NoahMP repository require the scheme to define its own physical constants, then we need to figure out a solution that works for everybody. One solution would be to define a NoahMP specific constants module. If using the CCPP, during the init phase of the scheme, we could pass in the physical constants from the host and set the constants in the NoahMP-specific constants module inside. If NOT using the CCPP, one could use pre-defined values in the NoahMP-specific module and not overwrite them in the init phase."

There are lots of schemes that define their own constants, and, in my opinion, I don't think that is a problem as long as the constants they are defining will ONLY ever be used within that scheme. If there is a chance that another scheme might use a given constant, then I think that it should be defined by the host model and passed in.

@grantfirl
Copy link
Collaborator

@climbfuji has previously provided the following example from the UGWP scheme to others dealing with the same thing:

"For an example, you can look at the ugwp code in the current NCAR ccpp-physics master repository, too:

https://github.com/NCAR/ccpp-physics/blob/master/physics/cires_ugwpv1_initialize.F90
https://github.com/NCAR/ccpp-physics/blob/master/physics/ugwpv1_gsldrag.F90 "

This provides an example of what I described in the previous post (although it is not in its own repository): using a scheme-specific constants module and setting them inside the init phase. If the scheme is simple enough (e.g. not very many subroutines), the constants can also just be passed in the run phase and propagated through the internal scheme code where necessary. For schemes with complex organization (like the UGWP scheme above), following the scheme-specific constants module method requires the fewest code/interface changes.

@grantfirl
Copy link
Collaborator

@dustinswales, do you have any input on this issue since you're maintaining your own repo for RRTMGP? That is, do other non-CCPP users of your repo expect physical constants (those that are potentially common to other schemes) to be defined internally? If so, how do you handle it?

@barlage
Copy link
Collaborator

barlage commented Mar 1, 2021

It is mentioned here:
https://github.com/earth-system-radiation/rte-rrtmgp/blob/33c8a984c17cf41be5d4c2928242e1b4239bfc40/rrtmgp/mo_rrtmgp_constants.F90#L12

but it would be good to know if this is used in CCPP.

@barlage
Copy link
Collaborator

barlage commented Mar 1, 2021

@grantfirl
Copy link
Collaborator

Yep, it looks like RRTMGP has the functionality that I described, but I'm not actually seeing anywhere in the CCPP interface to RRTMGP where mo_rrtmgp_constants%constants_init is actually called. But, that is a good example, if it were actually called.

@dustinswales It looks like constants are passed in to various RRTMGP-based interstitial schemes, but I'm not seeing that, for example, gravity internal to RRTMGP is set to the CCPP-provided one. Does this mean that RRTMGP is currently using its own value too? Please enlighten us when you get a chance.

@dustinswales
Copy link
Collaborator

@grantfirl
You are correct. GP is using its internal definitions. The init_constants() allows users to override the internal values. I should add a call to this init_constants during GP initialization, at least for gravity. @RobertPincus

@RobertPincus
Copy link
Collaborator

For context, RRTMGP uses four constants: Avogadro's number, the molecular weights of dry air and water vapor, and gravitational acceleration. These are used to compute the number of molecules per area from pressure and gas concentrations (i.e. to determine the opacity of the atmosphere). Making them consistent with the host model is a fine idea but maybe more important when we start using the UFS for exoplanets.

@grantfirl
Copy link
Collaborator

Thanks, gentlemen (@dustinswales and @RobertPincus). I have added an issue (#583) when you get a chance.

@climbfuji
Copy link
Collaborator Author

Thanks everyone, especially @grantfirl for continuing this discussion while I was out on the trails. I think we have a good way forward for schemes with and without their own repository.

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

No branches or pull requests

5 participants