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

Litter model theory and implementation docs #593

Merged
merged 41 commits into from
Oct 22, 2024

Conversation

jacobcook1995
Copy link
Collaborator

Description

This PR adds documentation for the litter model documentation of theory. Let me know a) if what I've added makes sense and b) if there's anything else that would be good to add.

The updated documentation can be viewed here https://virtual-ecosystem.readthedocs.io/en/513-litter-model-documentation/

@TaranRallings as part of this I added some stuff to the animal model docs, let me know if you are happy with that. I'm also completely have for you to rework it as you see fit when you write the animal model documentation.

Fixes #513

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • Make sure you've run the pre-commit checks: $ pre-commit run -a
  • All tests pass: $ poetry run pytest

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works
  • Relevant documentation reviewed and updated

@jacobcook1995 jacobcook1995 linked an issue Oct 14, 2024 that may be closed by this pull request
@davidorme
Copy link
Collaborator

@jacobcook1995 Partial edits. I've tried to do a couple of things:

  • Make more use of bullet points and lists to help make the text more structured.
  • I've moved all of the statements about future directions out of main text - where it tended to obscure what we are doing now - and into standalone admonitions.
  • Shorten the text a bit.

If those pushes seem ok, let me know and I'll tackle the last big file on - whatever it was...

@jacobcook1995
Copy link
Collaborator Author

@davidorme think it's a real improvement thank you! Only thing is that the :telescope: doesn't seem to change the admonition when the docs build and instead just becomes an extra part of the title

Copy link
Collaborator

@vgro vgro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how you describe the model and the references you provide for context. Your writing is very clear and easy to follow in most places.
I think the two models could be linked a bit better, it's not quite clear to me what happens where any why; for example why is there no environmental impact on the soil processes describes? Would it be worth starting with the litter model?

Here a few more specific comments:
Theory of biomass decay:

  • I am not sure about 'biomass decay' as the main heading, in other places you refer to 'soil' or 'soil microbes' I think this might be a bit confusing
  • I think it would be worth dropping above and belowground somewhere
  • in the third paragraph, the biggest uncertainty IS / the biggest uncertainties ARE . Also maybe better to say 'in the litter model', just to make clear which one you are talking about. And a small typo in the last sentence: the model PROVIDES
  • maybe worth mentioning the significantly less data for below ground in the last paragraph

Theory of the soil:

  • maybe add to first sentence a short overview of the content and link to headings
  • first paragraph: there seems to be a word missing: In response to this there has been a movement towards using soil carbon pool definitions that defined by measurable physical and chemical properties
  • could you say something about the relative contribution of the different carbon pools and the turnover rates? just roughly, is it daily or millennia?

Phosphorous pools

  • you make reference to rain forests in the first sentence, can you say something about other systems, too?

the section on microbial representation could be a bit more specific, i.e. mention functional groups or enzymes (or why you decided not to use it)

I think it would be useful to mention somewhere how these nutrients are moved from one pool to the next (going back to residence times and rates) and to other part of the model

Theory of litter decay

  • above ground litter: all losses ARE assumed to be from...

@davidorme
Copy link
Collaborator

Only thing is that the :telescope: doesn't seem to change the admonition when the docs build and instead just becomes an extra part of the title

I saw that and am sad about it... I wanted this: 🔭

@jacobcook1995
Copy link
Collaborator Author

@vgro I've made most of the changes you suggested. I haven't changed the description of the nitrogen and phosphorus pools, this is mainly because I am currently working on them and haven't settled on a final structure. I've made an issue (#606) to update this documentation later

slow decaying litter pools. This indirectly captures the impact of nitrogen and
phosphorus chemistry on litter decay.

The rest of this page gives provides details on the specific litter pools, the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed adjusting this to the new layout

Copy link
Collaborator

@vgro vgro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the changes that you made and I think it is in a good state as an intermediate overview. I think it's clear that this is not the final version from the intro.

@davidorme
Copy link
Collaborator

I think it's useful to have a common "in progress" admonition box for these pages? I mean, there's a chance they never go away, but then we can just have that at the top and keep the text cleaner. So:

In progress box

Text text text

Future directions box

@jacobcook1995
Copy link
Collaborator Author

@davidorme I've implemented your suggestions about admonitions, let me know what you think

Copy link
Collaborator

@TaranRallings TaranRallings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jacobcook1995 jacobcook1995 dismissed davidorme’s stale review October 22, 2024 12:37

David said he was happy with the changes, but I think forgot to approve

@jacobcook1995 jacobcook1995 merged commit a7efd83 into develop Oct 22, 2024
13 checks passed
@jacobcook1995 jacobcook1995 deleted the 513-litter-model-documentation branch October 22, 2024 12:57
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.

Litter model documentation
5 participants