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

remove HydrologyEarthParameters struct #982

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ ClimaLand.jl Release Notes
main
-------

- Change `Soil.phase_change_source` interface to take model parameters
as the final argument; remove `HydrologyEarthParameters` struct.
PR[#982](https://github.com/CliMA/ClimaLand.jl/pull/982)

v0.15.7
-------
Expand Down
10 changes: 4 additions & 6 deletions src/standalone/Soil/energy_hydrology.jl
Original file line number Diff line number Diff line change
Expand Up @@ -645,10 +645,6 @@ function ClimaLand.source!(
_ρ_i = FT(LP.ρ_cloud_ice(earth_param_set))
Δz = model.domain.fields.Δz # center face distance

# Wrap hydrology and earth parameters in one struct to avoid type inference failure. This is allocating! We should remove it.
hydrology_earth_params =
ClimaLand.Soil.HydrologyEarthParameters.(hydrology_cm, earth_param_set)

@. dY.soil.ϑ_l +=
-phase_change_source(
p.soil.θ_l,
Expand All @@ -666,7 +662,8 @@ function ClimaLand.source!(
),
ν,
θ_r,
hydrology_earth_params,
hydrology_cm,
params,
)
@. dY.soil.θ_i +=
(_ρ_l / _ρ_i) * phase_change_source(
Expand All @@ -685,7 +682,8 @@ function ClimaLand.source!(
),
ν,
θ_r,
hydrology_earth_params,
hydrology_cm,
params,
)
end

Expand Down
37 changes: 7 additions & 30 deletions src/standalone/Soil/soil_heat_parameterizations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ end
τ::FT,
ν::FT,
θ_r::FT,
hydrology_earth_params::HEP
) where {FT, HEP}
hydrology_cm::HCM,
params::P
) where {FT, HCM, P}
Returns the source term (1/s) used for converting liquid water
and ice into each other during phase changes. Note that
there are unitless prefactors multiplying this term in the
Expand All @@ -49,11 +50,10 @@ function phase_change_source(
τ::FT,
ν::FT,
θ_r::FT,
hydrology_earth_params::HEP,
) where {FT, HEP}
# Extract parameter sets from their container
hydrology_cm = hydrology_earth_params.hydrology_cm
earth_param_set = hydrology_earth_params.earth_param_set
hydrology_cm::HCM,
params::P,
) where {FT, HCM, P}
(; earth_param_set) = params

_ρ_i = FT(LP.ρ_cloud_ice(earth_param_set))
_ρ_l = FT(LP.ρ_cloud_liq(earth_param_set))
Expand All @@ -74,29 +74,6 @@ function phase_change_source(
return (θ_l - θstar) / τ
end

"""
struct HydrologyEarthParameters{
HCM <: AbstractSoilHydrologyClosure,
EP <: ClimaLand.Parameters.AbstractLandParameters,
}

A wrapper type around the hydrology closure model and land parameter
structs. This is needed because of a type inference failure coming from
ClimaCore when multiple structs and fields are broadcasted over.
This struct circumvents that issue by wrapping the structs in a single
type, that can be unpacked within the broadcasted function.

See github.com/CliMA/ClimaCore.jl/issues/2065 for more information
"""
struct HydrologyEarthParameters{
HCM <: AbstractSoilHydrologyClosure,
EP <: ClimaLand.Parameters.AbstractLandParameters,
}
hydrology_cm::HCM
earth_param_set::EP
end
Base.broadcastable(x::HydrologyEarthParameters) = tuple(x)

"""
volumetric_heat_capacity(
θ_l::FT,
Expand Down
81 changes: 14 additions & 67 deletions test/standalone/Soil/soil_parameterizations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,6 @@ for FT in (Float32, Float64)
vg_n = FT(1.4)
hcm = vanGenuchten{FT}(; α = vg_α, n = vg_n)

# Wrap hydrology and earth parameters in one struct to avoid type inference failure
hydrology_earth_params =
ClimaLand.Soil.HydrologyEarthParameters(hcm, param_set)

K_sat = FT(1e-5)
ν_ss_om = FT(0.1)
ν_ss_gravel = FT(0.1)
Expand Down Expand Up @@ -343,26 +339,12 @@ for FT in (Float32, Float64)
ρc_s = volumetric_heat_capacity.(θ_l, θ_i, parameters.ρc_ds, param_set)
τ = thermal_time.(ρc_s, Δz, parameters.κ_dry)
@test (
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) ≈ (θ_l .- θ_star) ./ τ
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) ≈
(θ_l .- θ_star) ./ τ
)
@test sum(
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) .> 0.0,
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) .>
0.0,
) == 3

θ_l = FT.([0.11, 0.15, ν])
Expand All @@ -378,15 +360,8 @@ for FT in (Float32, Float64)
ρc_s = volumetric_heat_capacity.(θ_l, θ_i, parameters.ρc_ds, param_set)
τ = thermal_time.(ρc_s, Δz, parameters.κ_dry)
@test (
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) ≈ zeros(FT, 3)
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) ≈
zeros(FT, 3)
)
@test (θ_star ≈ θ_l)

Expand All @@ -404,26 +379,12 @@ for FT in (Float32, Float64)
ρc_s = volumetric_heat_capacity.(θ_l, θ_i, parameters.ρc_ds, param_set)
τ = thermal_time.(ρc_s, Δz, parameters.κ_dry)
@test (
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) ≈ (θ_l .- θ_star) ./ τ
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) ≈
(θ_l .- θ_star) ./ τ
)
@test sum(
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) .< 0.0,
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) .<
0.0,
) == 2


Expand All @@ -440,26 +401,12 @@ for FT in (Float32, Float64)
ρc_s = volumetric_heat_capacity.(θ_l, θ_i, parameters.ρc_ds, param_set)
τ = thermal_time.(ρc_s, Δz, parameters.κ_dry)
@test (
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) ≈ (θ_l .- θ_star) ./ τ
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) ≈
(θ_l .- θ_star) ./ τ
)
@test sum(
phase_change_source.(
θ_l,
θ_i,
T,
τ,
ν,
θ_r,
hydrology_earth_params,
) .> 0.0,
phase_change_source.(θ_l, θ_i, T, τ, ν, θ_r, hcm, parameters) .>
0.0,
) == 2
end
end
Loading