CliMA / Land

Everything within the Land model (Soil Plant Atmosphere Module, Land Hydrology, etc)
https://clima.github.io/Land/
Apache License 2.0
98 stars 20 forks source link

Change to new clima parameter handling #53

Open charleskawczynski opened 2 years ago

charleskawczynski commented 2 years ago

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:

Yujie-W commented 2 years ago

This issue may trace back to an earlier open issue https://github.com/CliMA/Land/issues/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 commented 2 years ago

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 commented 2 years ago

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.