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

Revise the P Model documentation #441

Merged
merged 8 commits into from
Feb 28, 2025

Conversation

davidorme
Copy link
Collaborator

Description

This PR revises the contents of source/users/pmodel:

  • The shared components across PModel and SubdailyPModel are now broken out into a new directory.
  • The pmodel_details and subdaily_details part of the doc tree are now focussed on the specific implementations of those models.
  • Adds new diagrams of the calculation flow using the updated architecture. I'm experimenting with using draw.io diagrams here to get a zoomable diagram that will support tooltips.

Building the docs on this PR here (jumping to a page with an embedded draw.io to check they render on RTD and they do):

https://pyrealm.readthedocs.io/en/438-revise-the-p-model-documentation/users/pmodel/pmodel_details/pmodel_overview.html

Fixes #438

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

@davidorme davidorme linked an issue Feb 26, 2025 that may be closed by this pull request
@davidorme davidorme changed the base branch from develop to 383-meta-release-200 February 26, 2025 20:47
@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (383-meta-release-200@b72409e). Learn more about missing BASE report.

Additional details and impacted files
@@                   Coverage Diff                   @@
##             383-meta-release-200     #441   +/-   ##
=======================================================
  Coverage                        ?   96.72%           
=======================================================
  Files                           ?       35           
  Lines                           ?     2660           
  Branches                        ?        0           
=======================================================
  Hits                            ?     2573           
  Misses                          ?       87           
  Partials                        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@davidorme davidorme marked this pull request as ready for review February 27, 2025 15:51
{cite:p}`Prentice:2014bc,Wang:2017go,Stocker:2020dh`.

* The [subdaily P Model](subdaily_details/subdaily_overview), extends the P Model to
incorporates acclimation of photosynthetic pathways to changing environmental
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
incorporates acclimation of photosynthetic pathways to changing environmental
incorporate acclimation of photosynthetic pathways to changing environmental


* The relative advantages of the C3 and C4 photosynthetic pathways differ with
environmental conditions. The [C3 / C4 plant competition](c3c4model) model uses the
relative advantage to estimates the expected fraction of C4 plants in a community.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
relative advantage to estimates the expected fraction of C4 plants in a community.
relative advantage to estimate the expected fraction of C4 plants in a community.

{cite:p}`Prentice:2014bc,Wang:2017go` along with links to further details of the core
components of the model. It may be useful to read this alongside:

* The [worked examples](worked_examples) of using `pyrealm` to fitting the Standard P
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* The [worked examples](worked_examples) of using `pyrealm` to fitting the Standard P
* The [worked examples](worked_examples) of using `pyrealm` to fit the Standard P


The term $m_j$ is at the heart of the P model and describes the trade off between
carbon dioxide capture and water loss in photosynthesis. Given the environmental
conditions, a leaf will adjust its stomata to a value of ($\chi$) that optimises
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
conditions, a leaf will adjust its stomata to a value of ($\chi$) that optimises
conditions, a leaf will adjust its stomata to a value of $\chi$ that optimises

Comment on lines 90 to 92
this trade off. When $\chi$ is less than one, the partial pressure inside of
$\ce{CO2}$ inside the leaf is lowered and $m_j$ captures the resulting loss in light
use efficiency.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sentence somehow does not seem right. Is it really "inside of CO2 inside the leaf"?

* The form of the [Arrhenius scaling](./arrhenius) used to
calculate the electron transfer rate and carboxylation capacity at standard
temperatures. This is central to the Subdaily P Model as the variables $J_{max}$ and
$V_{cmax}$ need converted to a standard temperature to map predictions from the daily
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$V_{cmax}$ need converted to a standard temperature to map predictions from the daily
$V_{cmax}$ need to be converted to a standard temperature to map predictions from the daily

@davidorme davidorme merged commit ebfffab into 383-meta-release-200 Feb 28, 2025
16 checks passed
@davidorme davidorme deleted the 438-revise-the-p-model-documentation branch February 28, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Revise the P Model documentation
3 participants