-
Notifications
You must be signed in to change notification settings - Fork 2
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
Using pyrealm within the Plants model #707
Conversation
…nd add to PlantModel
…t all the same size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes - LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, a few points that we might want to discuss in person. One change that is unclear at the moment is what happened to the Cohorts?
@vgro Thanks for those comments - I've resolved the simple typos and doc updates and commented on the others. Could you review my responses and either resolve them if you are happy or flag them for more discussion! I've updated the docs to give more clarity on cohorts - they are part of the pyrealm community objects. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. The only remaining issue I have is the name of the canopy_absorption
which I think we should change if we include the radiation absorbed by the soil.
Description
This PR is to replace the placeholder internals of the Plants Model with actual pyrealm functionality. That is basically impossible to do in small steps, so this is an uncomfortably sprawling PR.
Tip
@vgro If you can look at the internals of the Plants Model setup and update to see if anything seems wrong
@sallymatson Could you look at the canopy, community and functional types to see if the way I'm using
pyrealm
structures is well enough described (for now, anyway).Fixes #659
Fixes #655
Type of change
Key checklist
pre-commit
checks:$ pre-commit run -a
$ poetry run pytest
Further checks