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

Change to new clima parameter handling #53

Open
charleskawczynski opened this issue Jun 27, 2022 · 3 comments
Open

Change to new clima parameter handling #53

charleskawczynski opened this issue Jun 27, 2022 · 3 comments

Comments

@charleskawczynski
Copy link
Member

If this repo is still being maintained, then we should apply the new clima parameter handling. An example of how to do this is in this PR.

A success metric for closing this issue involves:

  • Having a centralized module for where clima parameters from dependency packages can be accessed (see, e.g., CloudMicrophysics.jl's Parameters module, which forwards methods for handling thermodynamic variables).
  • Removing Land.jl's dependence on CLIMAParameters (i.e., using CLIMAParameters / import CLIMAParameters does not live in Land/src/).
  • All parameters that were previously obtained from CLIMAParameters methods are obtained from the CLIMAParameters toml file
@Yujie-W
Copy link
Collaborator

Yujie-W commented Jun 28, 2022

This issue may trace back to an earlier open issue #6.

The CLIMAParameters.jl was initially introduced to make sure all submodules within CliMA use the same sets of CONSTANTS. However, the recent PRs of CLIMAParameters.jl makes it harder and harder to use these CONSTANTS. Is the most natural way to define these constants once and use them everywhere? Why do we need to read a toml file while the CONSTANTS can be hard-coded?

Since the current design of CLIMAParameters.jl is diverging further and further from its original purpose, there is indeed no reason for me to use it any longer.

The way I handle the CONSTANTS is to have them defined in a Utility package I have (PkgUtility.jl), and then all CliMA Land submodules use it as a dependence. FYI, the constants are defined here. If users really want to change the CONSTANTS (for no reason), they can do something like this to overwrite the original definitions

import PkgUtility: LH_V0
LH_V0(FT) = FT(new value)

The CONSTANTS would change for the entire project.

Since I am not seeing CLIMAParameters.jl making it easier than the hard coded manner, I will remove the dependency on CLIMAParameters.jl completely in the next release. I am not planning to use CLIMAParameters.jl any more until it is easy to use (say only one line of code per CONSTANT).

As to the VARIABLES that are supposed to change spatially but not temporally, we are handling them through GriddingMachine.jl.

Does this address your question? @charleskawczynski

@tapios
Copy link

tapios commented Jun 28, 2022

The point of ClimaParameters is not just to ensure uniform values of planetary constants across all model components, although that too is important (e.g., to conserve energy in an Earth system model, we must make sure that latent heats are the same everywhere).

An additional purpose is to allow calibration of free parameters (including parameters appearing in neural networks, for example) in a uniform way. We want to be able to calibrate land and atmosphere (as one example) jointly with observational data; this is a fundamentally new and important aspect of what we do.

ClimaParameters allows us to do this.

Consider what you gain by using it: You instantly get access to a broad toolbox we have developed for learning about incompletely known model components from data. Just capture the relevant parameters in ClimaParameters (with prior information and transformations that ensure, e.g., positivity), and you can calibrate the model with whatever data you have available that may be informative about the model.

If you have specific suggestions for improving ClimaParameters, we'd like to hear them.

I think you will find making this change valuable for your own work. (cc @cfranken).

@Yujie-W
Copy link
Collaborator

Yujie-W commented Jun 28, 2022

An additional purpose is to allow calibration of free parameters (including parameters appearing in neural networks, for example) in a uniform way. We want to be able to calibrate land and atmosphere (as one example) jointly with observational data; this is a fundamentally new and important aspect of what we do.
ClimaParameters allows us to do this.

This is indeed new since CLIMAParameters.jl was introduced to us about 2 years ago when we were asked to use CLIMAParameters at the first place. I kind of remember someone pointed out ClimaConstants.jl would be a better name for the repository.

Anyway, if removing things like using/import CLIMAParameters is good, and also want an explicit way to list all CONSTANTS that are used within CliMA Land so that one can hack into these CONSTANTS. It is already done via a wrapper, and the values can be overridden as I mentioned in the last response. All CliMA Land sub-modules (not ClimaLSM.jl) use the CONSTANTS from the SAME utility package. The CONSTANTS are not stored in a struct at the present, but can be done in that way if you think that is more appropriate.

Regarding the broad toolbox, I agree with you that it is super valuable for research. And I am refactoring the Land model for the purpose. Hope I can get this done before the end of this year.

Yujie-W added a commit that referenced this issue Oct 31, 2022
53: add support to another colimit method r=Yujie-W a=Yujie-W



Co-authored-by: Yujie Wang <jesiner@gmail.com>
Yujie-W added a commit that referenced this issue Oct 31, 2022
53: fix leaf energy budget issue r=Yujie-W a=Yujie-W



Co-authored-by: Yujie Wang <jesiner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants