Initial draft of new quantum yield module

[davidorme]

Description

This PR implements a new approach to providing quantum yield (kphio) values to the PModel and SubdailyPModel.

• The kphio and do_ftemp_kphio arguments to PModel and SubdailyPModel are removed. These set a reference kphio value and then switched between constant or temperature modulated methods.
• They are replaced by method_kphio and reference_kphio. The method name links to a registry of subclasses of the new pyrealm.pmodel.quantum_yield.QuantumYieldABC, which provide different methods for calculating kphio.
• Each implementation just needs the definition of the _calculate_kphio abstract method, which has access to the PModelEnvironment object for the model and the shared constants definitions, as well as a flag setting whether C3 or C4 parameterisations should be used.
• Each implementation sets a default reference_kphio: the meanings of this reference vary with the method, so need to be documented.

In addition:

• The sandoval method introduced different derivations of the equation for calculating enzyme kinetics. The new function calc_modified_arrhenius_factor implements the two different equations and this function is now used with the QuantumYieldSandoval and replaces the calc_ftemp_inst_vcmax, which was basically a $V_{cmax}$ specific version of the function.

• The calc_ftemp_kphio function is replaced by QuantumYieldTemperature has been removed.
• The tests, docstrings and user facing docs have been updated to match.

Fixes #266

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)
• Breaking change

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

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 92.68293% with 12 lines in your changes missing coverage. Please review.

Project coverage is 95.06%. Comparing base (1f315ba) to head (16b45aa).
Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
pyrealm/pmodel/pmodel_environment.py	75.00%	4 Missing ⚠️
pyrealm/pmodel/quantum_yield.py	95.18%	4 Missing ⚠️
pyrealm/pmodel/subdaily.py	92.00%	2 Missing ⚠️
pyrealm/pmodel/functions.py	90.90%	1 Missing ⚠️
pyrealm/pmodel/pmodel.py	94.73%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #267      +/-   ##
===========================================
- Coverage    95.29%   95.06%   -0.23%     
===========================================
  Files           28       29       +1     
  Lines         1720     1802      +82     
===========================================
+ Hits          1639     1713      +74     
- Misses          81       89       +8     

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

[j-emberton]

I see you've added a csv of data related to the Sandoval k_phio approach.

Over time we're slowly increasing the size of the data stored in the build data folder. Is there a case for migrating this to a Pyrealm zenodo and downloading once for each test session? This would allow us to better manage test data provenance.

Alternatively, maybe we could just add a readme to the pyrealm_build_data/sandoval_k_phio folder to note where this data is sourced from for any future developers.

[davidorme]

@j-emberton and @AmyOctoCat This is now fully implemented - could you review.

We need to finalise what to do about the breaking change - we're thinking of releasing 2.0.0rc1 to give us some wiggle room on making the functionality available but not committing to the final state of a new 2.0.0 API.

[davidorme]

Alternatively, maybe we could just add a readme to the pyrealm_build_data/sandoval_k_phio folder to note where this data is sourced from for any future developers.

It's de novo generated from some appropriate inputs that are included in the folder.

Having said that, at the moment we have a page in the docs about the pyrealm_build_data package but that's separated from the package contents. It would actually make a lot more sense to move the documentation in there into docstrings in __init__.py files within the pyrealm_build_data package modules and then use autodoc to maintain an API like documentation tree for the package.

[j-emberton]

Alternatively, maybe we could just add a readme to the pyrealm_build_data/sandoval_k_phio folder to note where this data is sourced from for any future developers.

It's de novo generated from some appropriate inputs that are included in the folder.

Having said that, at the moment we have a page in the docs about the pyrealm_build_data package but that's separated from the package contents. It would actually make a lot more sense to move the documentation in there into docstrings in __init__.py files within the pyrealm_build_data package modules and then use autodoc to maintain an API like documentation tree for the package.

OK, lets not hold up this PR with doing that but perhaps add it as an issue

[j-emberton]

I think we should add this as an issue. It shouldn't be too difficult to have a compatibility check for any combination of selected methods. We could also add a quick reference to the docs for users.

If we create a v2.0 release milestone, then we can attach this issue to the v2.0 formal release and prioritise accordingly

[j-emberton]

Does this comment need to be in the docs?

[j-emberton]

I've added another couple of comments but no showstoppers. I think you're good to go :)

[j-emberton]

It just occurred to me that we possibly don't want to merge into develop, as that might then constrain us from further releases under the 1.x major version. Do we instead want to create a v2.0rc branch and maintain that in parallel until v2.0 release?