Add soil enzymes to model #341
Conversation
| placeholder: float = 123.4 | ||
| """A placeholder configurable constant.""" | ||
| depth_of_active_soil_layer: float = 0.25 | ||
| """Depth of the biogeochemically active soil layer [m]. |
There was a problem hiding this comment.
Based on previous discussions, we set the top soil layer to 50cm. Would it be worth changing that to get a better representation of the active zone?
There was a problem hiding this comment.
I also wonder if that is actually a core constant? It is very soil microbe specific
There was a problem hiding this comment.
The reason this is in core constants is because both the soil and litter models use it, but agree that it is confusing when the abiotic model also defined a soil layer height
There was a problem hiding this comment.
I've changed the default 1st soil layer height to -0.25m. I also made this issue to record our discussion, #348
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This looks overall good to me, I didn't look into the details of the tests.
We need to sit down and talk about the vertical flow units and definition to make sure we are on the same page. Other points to agree on are the temperature conversion (brings us back to the constants conversation and scipy) and the soil water retention curve parameter (which we both use).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #341 +/- ##
===========================================
+ Coverage 95.71% 95.91% +0.20%
===========================================
Files 51 52 +1
Lines 2541 2596 +55
===========================================
+ Hits 2432 2490 +58
+ Misses 109 106 -3 ☔ View full report in Codecov by Sentry. |
| pH=dummy_carbon_data["pH"], | ||
| bulk_density=dummy_carbon_data["bulk_density"], | ||
| soil_moisture=np.array([0.5, 0.7, 0.6, 0.2]), | ||
| soil_moisture=dummy_carbon_data["soil_moisture"][ | ||
| top_soil_layer_index |
There was a problem hiding this comment.
I thought you had replaced all soil_moisture with matric_potential, should we keep the soil moisture in for now? See #345
Description
This pull request changes the soil carbon model to make it an enzyme explicit model, i.e. one where carbon flows are controlled by enzymes pools which are included in the model. This is unfortunately quite a large pull request, as I couldn't find a sensible place to stop midway.
The largest structural change I've made is moving the calculations of environmental factors impact on biogeochemical rates into a separate submodule. I did this as
soil.carbonfelt like it was becoming too large.I've also started to make use of core constants as well as model constants. So feedback on how I've done this would be useful.
Generally looking for feedback on the structure, but if @vgro is able to check that the leaching function I've added is sensible that would be great.
Type of change
Key checklist
pre-commitchecks:$ pre-commit run -a$ poetry run pytestFurther checks