Using pyrealm within the Plants model#707
Conversation
…nd add to PlantModel
…t all the same size.
sallymatson
left a comment
There was a problem hiding this comment.
Thanks for making the changes - LGTM!
vgro
left a comment
There was a problem hiding this comment.
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. |
vgro
left a comment
There was a problem hiding this comment.
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
pyrealmstructures is well enough described (for now, anyway).Fixes #659
Fixes #655
Type of change
Key checklist
pre-commitchecks:$ pre-commit run -a$ poetry run pytestFurther checks