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

Define depth of first soil layer used by the abiotic (etc) models based on soil model constant #348

Closed
jacobcook1995 opened this issue Dec 4, 2023 · 7 comments

Comments

@jacobcook1995
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Currently the soil model defines a constant (CoreConsts.depth_of_active_soil_layer) which sets the depth of soil which is considered to be biogeochemically active, i.e. the depth of topsoil simulated in the soil model. The abiotic/hydrology models then also define soil layers separately (using config options).

@vgro pointed out that this is strange as we really want the first soil layer to correspond to the topsoil simulated by the soil model. At present the default options for both are set to 0.25m, but this still would allow users to select different values for the different models.

Describe the solution you'd like
I'm not 100% sure if we want to do this, but we could populate the topsoil layer height based on the core depth_of_active_soil_layer constant. This would have the advantage that the abiotic environmental information calculated for the 1st soil layer would always correspond to the layer that the soil model simulates. The drawback would be that this makes the system more inflexible.

@vgro
Copy link
Collaborator

vgro commented Dec 5, 2023

The depth of the soil layers is defined in the core schemer and initialized by the plant model. We could add the 0.25m as an additional layer or replace it; for the abiotic model it does not matter what the values are.

@davidorme
Copy link
Collaborator

I don't think it makes sense to have these two as separate. If soil_layers plausibly defines a different layering system to the active soil layer then we need to identify what the combination of those two systems is and configure that. If we are sticking with the two layers, then we could explicitly replace soil_layers with depth_of_active_soil_layer and bottom_of_soils (or something more technical 😄 ) That would probably be clearer about the configuration but does tie us to the two layer model (for now)

@jacobcook1995
Copy link
Collaborator Author

Or maybe depth_of_active_soil_layer and additional_soil_layers (a list), so that we are not tied into a strict 2 layer model. It does force the soil model to use a single layer, but I'm entirely comfortable with that

@davidorme
Copy link
Collaborator

What are we currently doing with additional soil layers - is that all hydrology?

@jacobcook1995
Copy link
Collaborator Author

I think so, the soil model doesn't touch them anyway

@jacobcook1995
Copy link
Collaborator Author

I think CoreConsts.depth_of_active_soil_layer should be renamed CoreConsts.depth_of_active_soil as it isn't a layer. Then a function can be written to extract reasonable averages for soil temp and moisture for the biologically active soil.

@jacobcook1995
Copy link
Collaborator Author

This has been addressed by the introduction of a layer structure. I'm closing this issue in favour of #532 and #533, which specifically set out the cleaning up that I need to do to move the litter and soil models to use the new system

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

No branches or pull requests

3 participants