-
Notifications
You must be signed in to change notification settings - Fork 46
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
Adjust potential NPP based on vegetation LUC losses (or gains) #639
Conversation
Cool, I am really interested to hear what @dawnlwoodard says since their work identified this problem. Something I am slightly confused about though is how the carbon pools are connected with the NPP. hector/src/simpleNbox-runtime.cpp Line 353 in d1af1b7
Based on my limited understanding of Hector's carbon cycle component it looks like the c pools (veg, detritus, and soil) are updated with luc emissions already right? So does this change now cause a double counting of the luc in these pools? Also does beta not have an effect on NPP anymore? |
Cool—thanks @kdorheim |
Old sequence:
New sequence:
|
Yay!! Thanks @bpbond! I will test this out tomorrow morning and plan to add the regrowth figures here so we can see how much of an impact this has. @kdorheim Yes the carbon pools are already updated with LUC emissions, but, unlike respiration, NPP does not itself depend on the size of the current carbon pools. It is purely calculated as a function of initial NPP and a CO2 fertilization multiplier. So this PR actually allows NPP to adjust with LUC emissions. Which, logically, should make some sense. If you cut down half of the forest, the NPP in that forest should not be the exact same as before. What we are trying to do here is something like this: where if Hector is in equilibrium and then you remove an amount of LUC emissions, Hector's land carbon drops to a new equilibrium. Of course the example I'm showing above doesn't have any nonlinear components, so Hector is not going to look this clean. And to maintain perfect equilibrium you also need to remove those LUC emissions proportional to the current size of each pool, but as Ben pointed out when we discussed this issue, that's not realistically how carbon would move, so we're also not going to do that. Also, beta should still affect NPP - this is just an additional multiplier. But at first glance this looks promising - as in, it does what we expect of increasing CO2 and dropping the size of the land carbon pools. |
Note that if you cut down half a forest, NPP doesn't necessarily change. It's just a different vegetation type. Any change would be because the new vegetation type has a lower or higher NPP than the old one. It could go either way. You need to look at the data to see which way that might go. But perhaps you mean that the NPP of the forest goes down? |
@ssmithClimate thanks, and right, it definitely can get tricky—NPP might not drop. But even if it didn't, the duplication-of-regrowth issue, along with the pave-the-amazon scenario (cf. #531 ), plus the fact that Hector already runs low compared to historical CO2/temp, all argue for addressing this I think. |
Hmmm @dawnlwoodard agreed that the soil C pool dynamics look funky @bpbond any ideas as to where the growth in soil C is coming from? |
Probably because we're not taking out the LUC pulse proportionately? I.e., we're still following Hector's default (more realistic, I said to you last week) fractions.
I suspect that this is a time-smudge issue: half is being taken out in 1800-1800.5, and half in 1800.5-1801, and because "1800.0" is January 1, 1800, it gets charged to two separate years. But yeah that's not what people will expect... Seems like I should make the first change, at least, so you have a clean basis for testing. |
@dawnlwoodard Temporarily shifted to proportional LUC losses/gains in 3b40525 ; haven't tested except yes it runs, but should do what you want. |
Thanks, @bpbond! So running with the changes and with nonlinear pieces turned off we would expect to produce a clean drop to a new equilibrium with no regrowth. But instead it now has consistent, small regrowth in every land C pool. It's a small amount, so not super concerned, but I am curious what's driving it. With the beta effect off, there's no actual dependence on the atmospheric carbon concentration, so it's not a response to that pool changing. With nonlinear back on (below) the jerkiness is fixed but the regrowth is still amplified somewhat (which we would expect). The question is just whether this is too much regrowth or not. Which, as @bpbond and I discussed when we first talked about this, is not necessarily a clear line to draw. My thought is that this is maybe small enough to not worry about if we can fix the regrowth that's happening in the purely linear one ^. Because this amplification doesn't seem like much if that weren't already happening. Mind you, none of what I've said here is necessarily a reason to not merge this in, as long as we can at least explain why that bit of regrowth is happening still in the purely linear version above. Like, less double counting is still an improvement. But imho if we're going to make a change this significant (in terms of impact on model results, necessitating recalibration, etc), then it does seem better to get it all the way how we want it. |
My other thought on this is that before you changed the luc fractions, @bpbond, the soil vs vegetation were not actually behaving in a more realistic way, unless I'm not thinking about this right. Based on what you and I discussed, what we talked about as being more realistic is that the soil should lose relatively less carbon due to LUC emissions, compared to vegetation and detritus. So what you'd expect is the soil carbon pool to drop further over time as it comes back into equilibrium, right? Whereas instead we saw the biggest drop in soil, such that it was regrowing, and vegetation and detritus were the ones dropping further to come into equilibrium. Am I missing something? Is that actually the behavior we'd expect? |
Fixed every test issue except for the old-new test; that'll be the last change, when everything else is resolved. Thanks @dawnlwoodard — interesting to see the graphs above! On the one hand, like you I am curious as to why, with concentration-driven mode and no beta and Q10=1, why there's adjustment over time in the carbon pools after your LUC pulse. I would guess that it takes time for the NPP-LUC effect to work its way through the system in some way we're not expecting. On the other, as you note, it's a small adjustment: <0.1%. Tough for me to prioritize this.
Hmm, let's touch base about this in person? I'm finding it hard to walk this through in my head. |
Sounds good, @bpbond. Let's discuss Wednesday. Re: "On the other, as you note, it's a small adjustment: <0.1%. Tough for me to prioritize this." - yes, the remaining effect here is small and alone I agree would not be a priority. But the intended default way we'd like to run Hector is of course with feedbacks on, which regrows a larger fraction of the carbon lost, and likely also with LUC fractions fixed, which regrows even more. I'm not sure we can just dismiss that amount of regrowth as insignificant. And it's unclear to me whether the fixes are unrelated or not. But, as I said before, less regrowth is still better. So we could certainly justify letting this be a good start and leaving it at that. |
@bpbond reminder to circle back to this. Per our discussion, we believe we are pulling too much carbon from all three land C pools relative to new NPP. Possibly an issue with calculations that happen outside of the solver versus fractions that get calculated in the solver. |
* Drop the T from RF_T_ALBEDO and expose LUC UPTAKE
Remove old `ffi()`, etc. functions; see #643
Fix interpolation behavior; add pulse test
@dawnlwoodard OK, I have fixed and merged ( #644 ) ANOTHER bug that your work has uncovered! 👏 The LUC pulse test now matches your spreadsheet I think — see graph in #643 . @kdorheim This PR now has some significant changes that, combined, definitely affect model behavior:
At your convenience, could you take a hard look at the performance of this PR, e.g. by running it through your v3 assessment RMarkdown you showed me yesterday? |
Two other notes before I go offline for a while.
LUC fractionsTo date Hector has let users set how LUC emissions (uptake) pull from (flow into) veg, detritus, and soil:
But I'm persuaded by Dawn's argument here: this approach is both ad hoc and dependent on what kind of LUC occurs, and it's simpler and more defensible to just pull proportionately from the three pools. That what was implemented in 3b40525 on a test basis, and I'm proposing we make permanent. Two questions then:
|
@bpbond will do taking a look at it now! |
@bpbond so exciting!! So glad you figured out what was behind the weird behavior I was seeing and that these tests can help with future diagnosis as well. Re: LUC fractions, I think it could be useful to have users be able to override them especially if they're biome-specific. Then someone could break out a particular biome that should have specific LUC-related behavior if they want. But if it's going to be a pain to keep them around, may not be worth it. |
@kdorheim glad CO2 is fitting better! Also, on these figures, is old luc just Hector v3 without Ben's latest fixes? I guess I'm particularly wondering why old and new luc have such different land sink behavior. |
@dawnlwoodard so there are two differences in the Hector output, the hand full of Ben's latest commits and also the parameters used. The difference between the land sink is a combination of these two factors. In the plot below I ran the same luc branch but with different parameter combinations and we see the large change in land sink behavior. old params
new params
|
I took an exploratory look at keeping Making these fractions biome-specific, and allowing for two different behaviors (default proportional, but user could override on a biome-specific basis), that has to be implemented in two different places ( So, my vote would be to remove them entirely, which is what #645 does. We can discuss next week, but if this makes sense, @kdorheim can merge #645 into this PR and then re-evaluate changed from 🙏 |
@bpbond in your figure generated above comparing Hector's outputs could you please include NBP, NPP, and Rh? |
Thanks Ben I will take a look at the PR in a bit. The terrestrial carbon cycle changes seem large do these changes seem reasonable/along the lines of what you were expecting? |
I don't know. Those NBP lines are pretty distressingly different. |
Yea... it seems like these changes have made Hector more sensitive to parameter values... What does the output looks like when |
hmmm interesting the developments seemed to have really increased Hector's parameter sensitivity |
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.
@bpbond this is only a partial review but may be relevant to the interpolation devlopment
@@ -2,6 +2,11 @@ | |||
; created by hectordata Tue Jan 11 10:27:08 2022 commit fd87739254cdc4bb18c6751d1d8707040a297d5d,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | |||
; UNITS:, PgC year-1, PgC year-1, PgC year-1, PgC year-1, Tg year-1, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppb, Tg CH4 year-1, ppm, Tg CO year-1, W/m2, W/m2, W/m2, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppt, ppb, Tg N year-1, Tg year-1, Tg NMVOC year-1, Tg N year-1, Tg year-1, ppt, ppt, Gg S year-1, W/m2, ppt, ppt, ppt, ppt, ppt, ppt | |||
Date,ffi_emissions,luc_emissions,daccs_uptake,luc_uptake,BC_emissions,C2F6_constrain,C2F6_emissions,CCl4_constrain,CCl4_emissions,CF4_constrain,CF4_emissions,CFC113_constrain,CFC113_emissions,CFC114_constrain,CFC114_emissions,CFC115_constrain,CFC115_emissions,CFC11_constrain,CFC11_emissions,CFC12_constrain,CFC12_emissions,CH3Br_constrain,CH3Br_emissions,CH3CCl3_emissions,CH3Cl_constrain,CH3Cl_emissions,CH4_constrain,CH4_emissions,CO2_constrain,CO_emissions,RF_misc,RF_albedo,RF_tot_constrain,HCFC141b_constrain,HCFC141b_emissions,HCFC142b_constrain,HCFC142b_emissions,HCFC22_constrain,HCFC22_emissions,HFC125_constrain,HFC125_emissions,HFC134a_constrain,HFC134a_emissions,HFC143a_constrain,HFC143a_emissions,HFC227ea_constrain,HFC227ea_emissions,HFC23_constrain,HFC23_emissions,HFC245_constrain,HFC245fa_emissions,HFC32_constrain,HFC32_emissions,HFC365_constrain,HFC365_emissions,HFC4310_constrain,HFC4310_emissions,N2O_constrain,N2O_emissions,NH3_emissions,NMVOC_emissions,NOX_emissions,OC_emissions,SF6_constrain,SF6_emissions,SO2_emissions,SV,halon1211_constrain,halon1211_emissions,halon1301_constrain,halon1301_emissions,halon2402_constrain,halon2402_emissions | |||
1745,0,0,0,0,0,0,0,0.025000429,0,34.04999924,0,0,0,0,0,0,0,0,0,0,0,5.299997807,0,0,457.0000025,0,731.4059957,0,277.1470032,0,0,0,0.286266211,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,273.8650513,0,0,0,0,0,0,0,0,0,0.004446573,0,0,0,0,0 |
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.
Why start at 1745? Is this a new requirement based on your recent developments?
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.
The model starts at 1745 🤷 otherwise no reason
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.
hahah that is a pretty good reason, I will update hectordata to make sure the input files start at year 1745
@@ -17,7 +17,6 @@ export(CH3BR_CONSTRAIN) | |||
export(CH3CCL3_CONSTRAIN) | |||
export(CH3CL_CONSTRAIN) | |||
export(CH4_CONSTRAIN) | |||
export(CO2_CONSTRAIN) |
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.
@bpbond did you intend to make these changes in the NAMESPACE file? I get the impression that it is a consequence of changing the formatting in the src/rcpp_constants.cpp
. Anyhow the overall effect removes all of these helper functions from the R code base which is not something I think we want to do. Unless do you have a reason why we would want to drop these functions then we should probably talk about it.
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.
Uh no whoops
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.
Thank you
Superseded by #651 |
Per discussion with @dawnlwoodard and @kdorheim yesterday — see #638
Changes
This PR adds a new term to our NPP calculation: vegetation losses (or gains) due to LUC change, with the the aim of avoiding double-counting regrowth fluxes. We save the end-of-spinup vegetation C value; keep a running total of LUC emissions from (or gains to) vegetation; and then adjust NPP:
Impact (SSP2.45)