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

Refactor Canopy BC #810

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Refactor Canopy BC #810

wants to merge 6 commits into from

Conversation

kmdeck
Copy link
Collaborator

@kmdeck kmdeck commented Oct 3, 2024

Purpose

We have recently refactored the boundary conditions for snow and soil (AtmosDriven...) to include a prognostic_land_component field, and we dispatch off of the Value of that field in order to correctly account for fluxes in the presence of other components in integrated models. This adds this to the canopy model.

To-do

Content

Major changes:

  1. Introduce "AtmosDrivenCanopyBC" which holds the atmos conditions, radiative conditions, and ground conditions. It also has a field for prognostic_land_components. This is more in line with how snow and soil hold these quantities. This changes the canopy model constructor and how we access atmos and radiation many places in the code.
  2. Renaming: Canopy.PrescribedSoil -> Canopy.PrescribedGroundConditions. The abstract type AbstractSoilDrivers -> AbstractGroundConditions also. The file soil_drivers.jl became ground_drivers.jl with this change.
  3. asdslad

Downstream or minor changes:

  1. As noted, Canopy.PrescribedSoil -> Canopy.PrescribedGroundConditions. The abstract type AbstractSoilDrivers -> AbstractGroundConditions also. Also, when it made sense, variables "_soil" became "_ground".
  2. We packaged (atmos, radiation, and ground <: AbstractGroundConditions) into a single AtmosDrivenCanopyBC. This is now in CanopyModel under boundary_conditions, instead of having each separate fields. This is more in line with how snow and soil hold these quantities. It also now has a field for prognostic_land_components. We check in the constructor that if typeof(ground) <: PrescribedGroundConditions, the only prognostic component is the canopy. And we check that if it is not Prescribed, soil must also be prognostic.
  3. The functions Canopy.ground_albedo_PAR now take as arguments both the "ground" driver (bc.ground, canopy.soil_driver in the previous main) and the prognostic_land_components. Currently this is primarily for consistency, but in the future (with snow) we may need to dispatch off the of Value of prognostic_land_components (snow, soil vs just soil).
  4. Similarly, root_energy_flux_per_ground_area!, root_water_flux_per_ground_area!, and canopy_radiant_energy_fluxes! also take not only the ground conditions (previously soil_driver) but also the prognostic_land_components. This is also to enforce consistency, but we can relax it if needed?
  5. Updated tests

  • I have read and checked the items on the review checklist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant