Adding Clima Thermodynamics#112
Conversation
Co-authored-by: Copilot <copilot@github.com>
…into ob/feature_clima_thermodyn
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
I now also see that there is an issue on the use of abbreviations in physical constants #111. Is this something to fix in this PR or not @bgroenks96 ? |
|
Hey @olivierbonte, I think this PR would be a good opportunity to resolve #111! I had also considered splitting up PhysicalConstants into separate types for thermodynamic vs material and hydraulic constants. What do you think about this? |
|
Alright! I'll try to address this soon I agree that splitting up the constants in categories makes sense, otherwise this one struct will get very large I think. Given the current set of properties, thermodynamic vs material makes sense to me. Any preferences for the naming of these structs? |
bgroenks96
left a comment
There was a problem hiding this comment.
Looks really great! Thanks a lot!
|
@olivierbonte Any updates on this? I guess once we merge this we could release a v0.1.1. |
|
Sorry for the delay on this @bgroenks96, had some deadlines for teaching responsibilities last week, but this week I'll pick this up again! |
bgroenks96
left a comment
There was a problem hiding this comment.
Oops, sorry forgot to submit these comments yesterday. I always find this "review" feature of GitHub a bit unintuitive.
|
@bgroenks96 I think all comments are addressed now, let me know what you think! |
| ) | ||
|
|
||
| # Always have `using Terrarium` available in doctests | ||
| DocMeta.setdocmeta!(Terrarium, :DocTestSetup, :(using Terrarium); recursive = true) |
There was a problem hiding this comment.
Nice, didn't know about this!
There was a problem hiding this comment.
What exactly does this do? I also haven't seen it.
There was a problem hiding this comment.
I think it inserts the given expression in every doctest so you don't need a @meta block everywhere.
bgroenks96
left a comment
There was a problem hiding this comment.
Looks good, great work! Just a few more comments, though, sorry 😅
| | `specific_heat_capacity_ice` | $c_{p,i}$ | 2070.0 | J/(kg·K) | Isobaric specific heat capacity of ice at 0°C | | ||
| | `specific_heat_capacity_liquid_water` | $c_{p,l}$ | 4186.0 | J/(kg·K) | Isobaric specific heat capacity of liquid water at 0°C | | ||
| | `specific_heat_capacity_water_vapor` | $c_{p,v}$ | 1846.0 | J/(kg·K) | Isobaric specific heat capacity of water vapor at 0°C | | ||
| | `latent_heat_fusion_at_reference` | $L_{sl}$ | 3.34×10⁵ | J/kg | Specific latent heat of fusion at 0°C | |
There was a problem hiding this comment.
Hmm, I get the inclination towards being precise by including _at_reference but I wonder whether or not this is really necessary? Can you imagine a situation where we would add more latent heat constants that would not be at a (or rather the) reference temperature?
| $FIELDS | ||
| """ | ||
| struct PhysicalConstants{NF} | ||
| thermodynamic_constants::ThermodynamicConstants{NF} |
There was a problem hiding this comment.
To be slightly less verbose, how about thermodynamics, material, and unviersal? That way it's constants.thermodynamics.specific_heat_capacity_dry_air. That would read a bit nicer, no?
|
|
||
| # Derived parameters | ||
| @inline Thermodynamics.Parameters.Rv_over_Rd(c::ThermodynamicConstants) = Thermodynamics.Parameters.R_v(c) / Thermodynamics.Parameters.R_d(c) | ||
| @inline ε(c::ThermodynamicConstants) = R_d(c) / R_v(c) |
There was a problem hiding this comment.
Can you make this explicitly spelled out? I guess it would be ratio_gas_constant_dry_air_to_water_vapor? A bit long but I guess OK?
There was a problem hiding this comment.
particularly important given that epsilon (not \varpesilon but still) is used for emissivity.
| Tₛ = skin_temperature(i, j, grid, fields, skinT) # skin temperature | ||
| Tₐ = air_temperature(i, j, grid, fields, atmos) # air temperature | ||
| Q_T = (Tₛ - Tₐ) / rₐ # bulk aerodynamic temperature-gradient | ||
| let rₐ = aerodynamic_resistance(i, j, grid, fields, atmos), # aerodynamic resistance |
There was a problem hiding this comment.
Can you revert the formatting change here? With Runic, I find it better not to use commas in the let block because otherwise it does this weird indenting here. We could also disable the formatter on all let blocks, but I think omitting commas is a cleaner solution. They aren't really necessary anyway.
There was a problem hiding this comment.
Actually you could also just remove the let block here since it's the only code in this function. I have a habit of using let blocks a lot, but it's admittedly kind of pointless in cases like this.
| Δq = compute_specific_humidity_difference(i, j, grid, fields, atmos, constants, Tₛ) | ||
| Q_h = Δq / rₐ # humidity flux | ||
| let L = constants.thermodynamic_constants.latent_heat_vaporization_at_reference, | ||
| Tₛ = skin_temperature(i, j, grid, fields, skinT), |
| let L = constants.Llg # specific latent heat of vaporization of water | ||
| ρₐ = constants.ρₐ # density of air | ||
| Q_h = surface_humidity_flux(i, j, grid, fields, evtr) # humidity flux | ||
| let L = constants.thermodynamic_constants.latent_heat_vaporization_at_reference, |
| @inline function compute_assimilation_factors( | ||
| photo::LUEPhotosynthesis{NF}, | ||
| constants::PhysicalConstants{NF}, | ||
| constants::MaterialConstants{NF}, |
There was a problem hiding this comment.
Yeah I think I like this! Much clearer dependency structure.
|
|
||
| """ | ||
| specific_humidity_to_vapor_pressure(q, p, ε) | ||
| vapor_pressure_to_specific_humidity(c::ThermodynamicConstants, e, pr) |
There was a problem hiding this comment.
Can you explain this change? Why does this need to be inverted now?
| """ | ||
| @kwdef struct MaterialConstants{NF} | ||
| "Density of water in kg/m^3" | ||
| water_density::NF = 1000.0 |
There was a problem hiding this comment.
I guess this should be density_water and density_ice if we want to be consistent with the pattern we have started following elsewhere of [quantity]_[constituent]_[location].
As previously mentioned in #108, I thought Thermodynamics.jl could be a nice way to avoid reimplementation of a lot of the thermodynamics. This PR is an attempt at replacing some utilities by functions from Thermodynamics.jl.
To avoid the use of ClimaParams.jl,
PhysicalConstantsis defined as a subtype ofAbstractThermodynamicsParameters. This approach is inspired by the use of Thermodynamics.jl in NumericalEarth.jl for the atmospheric thermodynamics (see here).This PR also introduces specific heat capacity and air density as functions, not constants, consistent with the use in Thermodynamics.jl