Feature/plants model #312
Feature/plants model #312
Conversation
…py area gets padded as expected
…draft estimate gpp method
… added test of dbh increment
| ground = np.where(self.data["layer_roles"].data == "surface")[0] | ||
| self.data["layer_absorbed_irradiation"][ground] = ground_irradiance | ||
|
|
||
| def estimate_gpp(self, time_index: int) -> None: |
There was a problem hiding this comment.
I'm slightly sceptical of estimate_gpp and allocate_gpp being included as methods of the PlantsModel, not so much from a code functionality point of view but feel like this could end up as a huge script if methods that low level are added. But that might be a misplaced fear if most of the functionality is farmed out to PyRealm
There was a problem hiding this comment.
That is my expectation - these should essentially extract numpy arrays from our Data instance and feed them into pyrealm functions. If they get larger, I will split them out into another submodule.
| for cohort in community: | ||
| cohort.gpp = np.nansum( | ||
| cohort.canopy_area * cell_gpp_per_m2 * seconds_since_last_update | ||
| ) |
There was a problem hiding this comment.
This is quite a long method that does quite a few things (or at least will do in future), I think it might be better to split it into simpler sub-functions. Though that can probably wait until the comments in it are filled out
There was a problem hiding this comment.
Agreed - it may actually get shorter!
| @@ -68,6 +89,8 @@ class PlantsModel(BaseModel): | |||
| vars_updated = ( | |||
| "leaf_area_index", # NOTE - LAI is integrated into the full layer roles | |||
| "layer_heights", # NOTE - includes soil, canopy and above canopy heights | |||
| "layer_fapar", | |||
| "layer_absorbed_irradiation", | |||
| "herbivory", | |||
| "transpiration", | |||
| "canopy_evaporation", | |||
There was a problem hiding this comment.
Is the distinction here that transpiration is water from the canopy interception pool and canopy_evaporation is the water that plants such out of the soil? If so, I will update the input to the hydrology model to canopy_evaporation to account for root water uptake. Also, is this going to be an accumulated values over all layers?
There was a problem hiding this comment.
The other way around, isn't it? Transpiration should be the stomatal conductance rate scaled up to the amount of photosynthesis and canopy_evaporation is water loss from the (currently non existent) canopy interception pool. I've not implemented any of that though, so if there is something better to do, let me know!
I can do transpiration per canopy layer - the stomatal conductance is a prediction of the P Model, which will be run within layers. In fact, I'll have to - but can always sum it if we don't need the vertical profile. For the evaporation - if we have a layer specific interception model, then that too! I think that's another PR though.
There was a problem hiding this comment.
yes, sorry, I mixed that up, I did mean transpiration.
I have implemented a simple interception pool for the whole canopy (then I got carried away and changed a million other little things and it's now a complete overhaul of the hydrology model, which you will soon see...), so we could include it in one of the future PRs in the hydrology model, I don't think it necessarily needs to touch the plant model because it only needs to LAI.
There was a problem hiding this comment.
It's also only relevant for us if we want to close the water balance
There was a problem hiding this comment.
I've made some small suggestions in a few places. In particular, you want to make sure to use pytest.approx for doing floating-point comparisons in unit tests, otherwise you can get spurious test failures if the data change subtly or you run them on different hardware.
Codecov Report
@@ Coverage Diff @@
## develop #312 +/- ##
===========================================
+ Coverage 96.97% 97.05% +0.08%
===========================================
Files 52 52
Lines 2346 2513 +167
===========================================
+ Hits 2275 2439 +164
- Misses 71 74 +3
... and 3 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Yeah - working on those! |
Description
This PR further extends the Plants Model to add the final (well, we can dream) parts of the API for modelling plant growth. None of the functions do any actual science yet, but the required inputs and internal variables are set up and the flow of the model is complete.
The PR:
Cohortobject to track the canopy area of members within the canopy layer and the gross primary productivity.Type of change
Key checklist
pre-commitchecks:$ pre-commit run -a$ poetry run pytestFurther checks