SIP180 Adding N demand, fixation, and uptake#265
Conversation
|
PR265 Smoke Test Output.txt Smoke and unit test output. Updated smoke test output file after pulling and merging from master. |
Alomir
left a comment
Alomir
left a comment
There was a problem hiding this comment.
Looks great! One doc comment, otherwise just need to get the PR up to date and make sure everything passes.
|
|
||
| \begin{equation} | ||
| F^N_\text{demand}=\frac{dN_\text{plant}}{dt} = \sum_{i} \frac{dN_{\text{plant,}i}}{dt} | ||
| F^N_\text{demand} = \sum_{i} \frac{F^C_{\text{creation,}i}}{CN_{\text{i}}} |
There was a problem hiding this comment.
I think this might be better doc'd as the "creation demand" (or some such), and separately the "leaf on event demand", and then total demand = sum of those two. Putting leafOnCreation in this equation - esp. without the subtraction term here.
Actually, if you want to dump explaining leaf-on here and create a ticket to do so, that would be fine - esp. given that there's a 'TODO' above to explain leaf-on in the first place up in the Leaf Carbon section. (If you do make a ticket, mention that too.)
There was a problem hiding this comment.
So separate the equation into creation demand + leaf on event demand, remove the little blurb about leaf on handling, and create a separate issue related to the TODO in leaf carbon?
There was a problem hiding this comment.
I went ahead and separated the N demand calculation in sipnet.c into leafOnDemand, creationDemand, and totalDemand. I removed the blurb about leaf on calculation and added a TODO note to possibly add a better explanation. I also opened Issue #275 to update documentation for leaf on/leaf off mechanics.
Merging documentation changes into branch
Alomir
left a comment
Alomir
left a comment
There was a problem hiding this comment.
Looks great! Just need to resolve conflicts, then it is good to go.
| Each term in the sum is calculated according to \eqref{eq:plant_n}. Total plant N demand $F^N_{\text{demand,}total}$ is then partitioned between fixation and soil N uptake using \eqref{eq:n_fix_demand} and \eqref{eq:n_uptake_demand}. | ||
|
|
||
| **TODO:** possibly include more context about leaf on events |
| /*! | ||
| * Calculate plant N demand | ||
| */ | ||
| double calcPlantNDemand() { |
2 participants
Summary
Changes that calculate plant N demand, N fixation, and N uptake fluxes. minN pool updated, fixation and uptake added as output columns, unit test created, new parameters created, balance tracker updated. These changes are motivated by Issue #180 to continue implementation of N cycle.
How was this change tested?
Reproduction steps
N/A
Related issues
Checklist
docs/CHANGELOG.mdupdated with noteworthy changesclang-format(rungit clang-formatif needed)Note: See CONTRIBUTING.md for additional guidance. This repository uses automated formatting checks; if the pre-commit hook blocks your commit, run
git clang-formatto format staged changes.