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

Aligning PModel and Subdaily PModel #401

Merged
merged 17 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ repos:
- id: debug-statements
- repo: https://github.com/astral-sh/ruff-pre-commit
# Ruff version.
rev: v0.9.3
rev: v0.9.4
hooks:
# Run the linter.
- id: ruff
Expand Down
23 changes: 23 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,29 @@ A new major release is planned but will iterate through release candidates in or
make functionality available for testing while the new functionality and API changes are
worked through. The changes below are provisional.

- The `PModel` and `SubdailyPModel` classes have been extensively restructured to align
the attributes and methods and to remove repeated code. Many of these changes are
internal but the model signatures have changed and several of the attributes have been
renamed.

**Breaking changes**:

- The `SubdailyPModel` attributes giving actual predicted estimates of $V_{cmax}$ and
$J_{max}$ for observations (`subdaily_vcmax`, `subdaily_vcmax25`, `subdaily_jmax`
and `subdaily_jmax25`) have been renamed to simply `vcmax`, `vcmax25`, `jmax` and
`jmax25` to align with the observation estimates in `PModel`.
- The `SubdailyPModel` attributes giving the daily optimum and realised values for
$V_{cmax25}$, $J_{max25}$ and $\xi$ have been renamed for more clarity: `vcmax25_opt`,
`vcmax25_real`, `jmax25_opt`, `jmax25_real`, `xi_opt` and `xi_real` have changed to
`vcmax25_daily_optimal`, `vcmax25_daily_realised`, `jmax25_daily_optimal`,
`jmax25_daily_realised`, `xi_daily_optimal` and `xi_daily_realised`.
- The `PModel.estimate_productivity` method was required to pass in FAPAR and PPFD to
scale up LUE predictions to GPP and to estimate other predictions. This has been
deprecated with addition of FAPAR and PPFD to the `PModelEnvironment` and GPP is now
calculated automatically.
- The `convert_pmodel_to_subdaily` function has been deprecated in favour of the new
`PModel.to_subdaily` method.

- The `PModelEnvironment` class has been updated. It now requires that the user also
provides `fapar` and `ppfd` data, currently with no default values. The provision of
additional variables has also been made more flexible, allowing users to provide
Expand Down
4 changes: 4 additions & 0 deletions pyrealm/pmodel/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
calculate_simple_arrhenius_factor,
)
from pyrealm.pmodel.isotopes import CalcCarbonIsotopes

# from pyrealm.pmodel.new_pmodel import PModelNew, SubdailyPModelNew
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather delete than comment out

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are as a placeholder to remind me when I strip out the old implementations. Because this isn't merging into develop directly, there are a few loose ends which I'll tidy up before that branch merges down.

from pyrealm.pmodel.pmodel import PModel
from pyrealm.pmodel.pmodel_environment import PModelEnvironment
from pyrealm.pmodel.scaler import SubdailyScaler
Expand All @@ -62,6 +64,8 @@
"PModel",
"PModelEnvironment",
"SubdailyPModel",
# "PModelNew",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same, rather delete

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here too.

# "SubdailyPModelNew",
"SubdailyScaler",
"calc_co2_to_ca",
"calc_ftemp_inst_rd",
Expand Down
Loading