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

Overhaul approach to physical constants and model parameters #86

Closed
white-alistair opened this issue May 27, 2022 · 2 comments · Fixed by #241
Closed

Overhaul approach to physical constants and model parameters #86

white-alistair opened this issue May 27, 2022 · 2 comments · Fixed by #241
Labels
naming 👶 Maybe there's a better name for it? structure 🏠 Internal structure of the code

Comments

@white-alistair
Copy link
Member

Currently, there is not a clear distinction between universal physical constants, which never change, and model parameters, which might.

Copying @milankl's comment from PR #82:

What about using "constants" for actual physical constants that nobody would want to change, "parameters" for everything that run_speedy may take as an argument and "constants_runtime" for every value that has to be precalculated, probably from parameters, but is constant at runtime. Meaning we should rename the constants.jl file and constants.jl should then be loaded before default_parameters?

@milankl milankl added naming 👶 Maybe there's a better name for it? structure 🏠 Internal structure of the code labels Jul 13, 2022
@white-alistair
Copy link
Member Author

white-alistair commented Aug 26, 2022

This topic came up again in #128.

With a growing number of parameters and constants, in particular in relation to the parameterizations, the various structs and constantly unpacking them is becoming a bit messy.

@milankl suggested an example structure like (OUTDATED see below)

struct Constants
    time_integration::TimeConstants
    parametrization::ParametrizationConstants
end

struct ParametrizationConstants
     clouds::CloudConstants
     convection::ConvectionConstants
end

as well as, potentially, some kind of @unpack_all macro for importing a specific struct of macros into a namespace.

In addition, we need to move some numbers from the Parameters struct into the Constants struct in order to avoid type promotions: #128 (comment)

Let's use this issue to do a general overhaul of how we use parameters and constants.

@white-alistair white-alistair changed the title Distinguish clearly between physical constants and model parameters Overhaul approach physical constants and model parameters Aug 26, 2022
@white-alistair white-alistair changed the title Overhaul approach physical constants and model parameters Overhaul approach to physical constants and model parameters Aug 26, 2022
@milankl milankl added this to the v0.5 milestone Sep 2, 2022
@milankl
Copy link
Member

milankl commented Feb 7, 2023

Picking this up again, with #241 we'll have the following

abstract type AbstractDynamicsConstants{NF} end
abstract type AbstractParameterizationConstants{NF} end

struct DynamicsConstants{NF<:AbstractFloat} <: AbstractDynamicsConstants{NF}
struct ParameterizationConstants{NF<:AbstractFloat} <: AbstractParameterizationConstants{NF}

While it sounds tempting to group both into Constants or something, i don't think it makes sense as BarotropicModel or ShallowWaterModel won't use ParameterizationConstants so we should keep them separate. I'm still in favour of the following distinction between parameters, coefficients, constants. While all are * constant * at runtime

  • parameters are user-facing, have a default value but can be changed through arguments to initialize_speedy, and define together with PrognosticVariables the model run completely.
  • coefficients are bundled parameters, to be used (mostly) within the same function/algorithm; e.g. we have the MagnusCoefficients for the calculation of saturation vapour pressure
  • constants face the dynamical core / parameterizations, are fully determined by all parameters/coefficients, may include some precalculation (e.g. the scaled time step which is not chosen directly as parameter) and conversion to a given number format.

So schematically we have

  • user -> parameters + coefficients -> constants -> dynamical core / parameterizations

@milankl milankl linked a pull request Feb 7, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
naming 👶 Maybe there's a better name for it? structure 🏠 Internal structure of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants