Conversation
|
|
||
| # Update the canopy radiation | ||
| canopy_radiant_energy_fluxes!( | ||
| canopy_shortwave_fluxes!( |
There was a problem hiding this comment.
This is called in the function that updates all of the cache (used prior to computing the explicit tendency)
There was a problem hiding this comment.
Not all of these fluxes are needed in the explicit tendency, but we have made it so "update aux + update boundary fluxes" updates the whole cache at the beginning of each step (and is called 1x per explicit tendency computation).
Then there is a smaller subset of the cache which is the implicit cache and that is updated only via update_implicit* functions
There was a problem hiding this comment.
Would it be possible to instead update only the explicit components in "update aux + update bf"? I.e. remove the canopy_longwave_fluxes! call here?
| # Update the canopy radiation | ||
| canopy_radiant_energy_fluxes!( | ||
| # Update the canopy LW radiation | ||
| canopy_longwave_fluxes!( |
There was a problem hiding this comment.
this is part of the update implicit boundary fluxes for canopy energy
| """ | ||
| @kwdef struct SoilSublimation{FT} <: AbstractSoilSource{FT} | ||
| explicit::Bool = false | ||
| explicit::Bool = true |
There was a problem hiding this comment.
this is handled explicitly now, forgot to change in earlier PR (all soil boundary fluxes are explicit)
| @kwdef struct SoilSublimationwithSnow{FT} <: | ||
| ClimaLand.Soil.AbstractSoilSource{FT} | ||
| explicit::Bool = false | ||
| explicit::Bool = true |
There was a problem hiding this comment.
All soil fluxes at the boundary are explicit - this was not updated in a previous PR which meant that it was included as part of the implicit tendency (not no contribution to Jacobian)
|
|
||
| Computes and sets the net shortwave radiation for the snow and soil. | ||
| """ | ||
| function update_radiation_fluxes!(p, land::SoilSnowModel{FT}, Y, t) where {FT} |
There was a problem hiding this comment.
this moves the radiative fluxes for integrated models outside of the "soil_boundary_fluxes" and "snow_boundary_fluxes" function. It is more in line with what we do when there is a canopy
There was a problem hiding this comment.
though not necessary in this case, I change it to align with the other integrated models
| in advance of this function being called. | ||
| """ | ||
| function lsm_radiant_energy_fluxes!( | ||
| function update_radiation_fluxes!( |
| end | ||
|
|
||
| """ | ||
| implicit_radiation_fluxes!(p, land::LandModel{FT}, |
There was a problem hiding this comment.
updates subset of radiative fluxes which are treated implicitly
juliasloan25
left a comment
There was a problem hiding this comment.
This looks good! One thing I find a little confusing is why update_aux/update_boundary_fluxes still update both the explicit and implicit variables. E.g. could we replace update_radiation_fluxes with explicit_radiation_fluxes to mirror implicit_radiation_fluxes?
| bc = model.boundary_conditions | ||
|
|
||
| SW_net = @. lazy((p.snow.α_snow - 1) * p.drivers.SW_d) #match sign convention in ./shared_utilities/drivers.jl | ||
| net_sw_radiation!(p.snow.SW_n, bc.radiation, model, Y, p, t) |
There was a problem hiding this comment.
Are SW and LW both still computed in this function because everything is explicit for standalone snow?
| - the bulk snow density (`ρ_snow`, kg/m^3) | ||
| - the SHF, LHF, and vapor flux (`turbulent_fluxes.shf`, etc), | ||
| - the net radiation (`R_n, J/m^2/s)`, | ||
| - the net LW and SW radiation (`LW_n, SW_n, J/m^2/s)`, |
There was a problem hiding this comment.
| - the net LW and SW radiation (`LW_n, SW_n, J/m^2/s)`, | |
| - the net LW and SW radiation (`LW_n, SW_n`, J/m^2/s), |
to match the other entries here
|
|
||
| # Update the canopy radiation | ||
| canopy_radiant_energy_fluxes!( | ||
| canopy_shortwave_fluxes!( |
There was a problem hiding this comment.
Would it be possible to instead update only the explicit components in "update aux + update bf"? I.e. remove the canopy_longwave_fluxes! call here?
| f_abs_par = p.canopy.radiative_transfer.par.abs | ||
| f_abs_nir = p.canopy.radiative_transfer.nir.abs | ||
| @. p.canopy.radiative_transfer.SW_n = f_abs_par * par_d + f_abs_nir * nir_d | ||
| @. p.canopy.radiative_transfer.SW_n = |
There was a problem hiding this comment.
why did you change the signs?
| @test all(parent(p.snow.total_water_flux) .≈ 0) | ||
| # Make sure the boundary conditions match bare soil result | ||
| set_soil_initial_cache! = make_set_initial_cache(land_model.soil) | ||
| set_soil_initial_cache! = make_set_initial_cache(soil_model) |
There was a problem hiding this comment.
why not continue using land_model.soil?
| function update_radiation_fluxes!(p, land::SoilSnowModel{FT}, Y, t) where {FT} | ||
| snow = land.snow | ||
| soil = land.soil | ||
| radiation = snow.boundary_conditions.radiation |
There was a problem hiding this comment.
| radiation = snow.boundary_conditions.radiation | |
| radiation_snow = snow.boundary_conditions.radiation |
to distinguish from soil radiation
Purpose
In previous PRs, we made the following design choice:
"update cache" calls update aux for a model + update boundary fluxes for a model. It updates everything, regardless of if it is implicit or explicit.
"update implicit cache" updates implicit aux + implicit fluxes for a model ONLY.
Each step looks like this then:
The exception was with radiative fluxes. We only had a single function which computed SW and LW fluxes, but SW fluxes are treated explicitly, and only canopy LW is treated implicitly.
For the canopy alone this was fine because canopy SW is independent of state. This is not the case in integrated models. For soil alone this was fine because everything was explicit, but this is not the case with integrated models with a canopy.
This led to an issue where we were overwriting the soil net radiation (for example) in the cache using canopy and soil temperatures at the next step, though the value at the previous step (explicit) was used to update the soil boundary conditions. This is not correct because then the value in the cache - which is used to set BC for the atmosphere - doesnt reflect what the soil actually saw.
We can correct this error by more clearly labeling LW as implicit and only updating it in the implicit boundary flux function. This PR splits up SW and LW computations.
It also moves the soil/snow integrated radiation computation outside of the component model's update boundary flux function. This makes it match the other integrated models (with a canopy), which require the "integrated model" to compute radiative transfer rather than computing each components net radiation individually.
To-do
Content