SIP86 Add soil mineral nitrogen pool and nitrogen cli option - #171
Conversation
dlebauer
left a comment
dlebauer
left a comment
There was a problem hiding this comment.
Looks good, excited to begin the N cycle modeling journey! A few comments, no major changes required.
|
|
||
| Note the following restrictions on these options: | ||
| - `soil-phenol` and `gdd` may not both be turned on | ||
| - `events` and `microbes` may not both be turned on |
There was a problem hiding this comment.
Should we also prohibit nitrogen + microbes both on? I suspect it would be complicated and at this point unnecessary to support the combination.
| | 24 | $R$ | rtot | Total ecosystem respiration (ra + rh) | g C m$^{-2}$ | | ||
| | 25 | | evapotranspiration | ET (transpiration + immedEvap + evaporation + sublimation) for timestep | cm | | ||
| | 26 | | fluxestranspiration | Transpiration component only | cm | | ||
| | 27 | | minN | Soil mineral nitrogen | g N m$^{-2}$ | |
There was a problem hiding this comment.
| | 27 | | minN | Soil mineral nitrogen | g N m$^{-2}$ | | |
| | 27 | $N_\textrm{min}$ | minN | Soil mineral nitrogen | g N m$^{-2}$ | |
| initializeOneModelParam(modelParams, "microbePulseEff", &(params.microbePulseEff), ctx.microbes ); | ||
| initializeOneModelParam(modelParams, "microbePulseEff", &(params.microbePulseEff), ctx.microbes); | ||
|
|
||
| // Nitrogen cycle params for the MAGIC project |
There was a problem hiding this comment.
Is a reason to identify this as a MAGiC component? Seems restrictive.
There was a problem hiding this comment.
@dlebauer My goal is to give some attribution to these changes, since we don't have a publication to point to. Can you think of a better wording suggestion?
| // [3] Zobitz (et al.?), date unknown, chapter 5 from unknown book | ||
| // [4] Zobitz (et al.?), date unknown, chapter 5 from unknown book | ||
| // | ||
| // Also of note, additions have been made to support the MAGIC |
There was a problem hiding this comment.
We could use LeBauer et al (unpublished) "SIPNET 2: A lightweight, extensible model for coupled C–N–H₂O–GHG dynamics in managed ecosystems" as a reference for now.
There was a problem hiding this comment.
Ah, nice! Ignore previous comment :-)
| fineRootExudation 0.01 0 0 1 0.001 | ||
| coarseRootExudation 0.01 0 0 1 0.001 | ||
| microbePulseEff 0.45 0 0 1 0.05 | ||
| mineralNInit 1.0 0 0 0 0 |
There was a problem hiding this comment.
Not urgent or required for this pr.
But, didn't we change it so this file only requires the first value? Even if the legacy format is still supported, should we update tests to reflect the version without the unused cols?
There was a problem hiding this comment.
Probably, though I might prefer that as a separate ticket? Let me take a quick shpt, if it takes more than a few minutes I'll spawn it off.
There was a problem hiding this comment.
I'm not sure what a shpt is but definitely not urgent. Just caught my attention.
2 participants
Fixes #86
This PR:
envivariable,minN, to track the soil mineral nitrogen poolminNInit, to initial that poolminNtosipnet.outin a new column labeledminNnitrogen-cycle, that controls whetherminNis used/updated.russell_2smoke test as a new nitrogen-cycle smoke test to verify that the above worksmodel-inputs.mdandmodel-outputs.md