ImperialCollegeLondon / virtual_ecosystem

This repository is the home for the codebase for the Virtual Ecosystem project.
https://virtual-ecosystem.readthedocs.io
BSD 3-Clause "New" or "Revised" License
9 stars 1 forks source link

Added tracking of litter nitrogen #531

Closed jacobcook1995 closed 1 month ago

jacobcook1995 commented 1 month ago

Description

This pull request adds tracking of the nitrogen concentration for each litter pool, as well as a calculation of the rate of nitrogen mineralisation into soil. It involves a slightly weird assumption that the litter flow to a metabolic litter pool always has 5 times the nitrogen concentration of the flow to the corresponding structural pool. While this is a bit weird it seems to be used fairly commonly across existing as a simplifying assumption, so I think this is an issue best left for a post-1.0 "lets improve how we model litter decay" rework (if we decide that's a priority).

Structurally the biggest change is the addition of a LitterChemistry class, which contains methods to track the changes in litter chemistry (currently lignin and nitrogen). This is partly to make extending the code to also track phosphorus simpler, but is also intended to simplify the flow of LitterModel.update(). This simplification is still a bit of a work in process, as I didn't change how input partitioning is handled as I haven't worked out what I'm doing with animal inputs yet (see #510).

Fixes #511

Type of change

Key checklist

Further checks

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 96.80851% with 3 lines in your changes missing coverage. Please review.

Project coverage is 94.97%. Comparing base (0cf635a) to head (7ce708f).

Files Patch % Lines
...ple_data/generation_scripts/litter_example_data.py 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #531 +/- ## =========================================== + Coverage 94.96% 94.97% +0.01% =========================================== Files 73 74 +1 Lines 4071 4141 +70 =========================================== + Hits 3866 3933 +67 - Misses 205 208 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jacobcook1995 commented 1 month ago

The PR is a bit on the long side, but I guess it is not easy to do it in small chunks. Something we have done in other projects is to add new code in small chunks without hooking it into the main pipeline (eg. just writing standalone functions and classes bit by bit, alongside their tests), and then have a final PR that hooks everything together into the main execution workflow, refactoring the parts that need refactoring but without adding big chunks of code. Could be something to consider for the future.

Yeah I think this is a fairly classic case of me not anticipating how much would change downstream, I'll try and follow your suggested split when I add the litter phosphorus