-
Notifications
You must be signed in to change notification settings - Fork 24
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
Revision of emisCO2.R #67
Conversation
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.
plots look good to me. Looking at the code I was wondering whether this one is now really linter free as I saw some snake_cases showing up and there might be other issues. Please make it linter free and also build the package to add and updated DESCRIPION file and so on.
We've now corrected a small bug in the emissions from forestry, wherein ndc and aff plantations were sometimes subject to harvesting. Now, we can integrate the check that gross emissions have to be greater than zero. |
.validateCalculation <- function(totalStock, totalStockCheck, output) { | ||
|
||
# --- Ensure independent output of carbonstock is nearly equivalent to own calculation | ||
if (any(totalStock - totalStockCheck > 1e-05, na.rm = TRUE)) { |
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.
Kristine, this check had to be 1e-05. Florian suggested that the issue could be someplace here: https://github.com/magpiemodel/magpie/blob/master/modules/59_som/cellpool_jan23/presolve.gms.
However, since it's still such a small inconsistency, it doesn't warrant too serious an investigation.
R/reportEmissions.R
Outdated
|
||
# remove negative emissions from long-term storage of wood products | ||
harvestOld <- dimSums(co2[, , "lu_harvest"], dim = 3) | ||
harvestNew <- harvestOld - (emisWoodInflow + emisBuildingInflow) |
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.
Florian, could you check this logic? If my interpretation is correct, then this is all ready to push.
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.
I think emisWoodInflow
and emisBuildingInflow
are negative (previously I thought they are positively defined)
Therefore it should be + and not -
but better check by running the carbonLTS function
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.
Good. That resolves my confusion, thank you. I will push it all ASAP.
Jan, I made it lintr friendly with two exceptions, one is a cyclic comp lintr that I am going to nolint (but it wasn't working so I asked Pascal and will update later). The other is commented code from the agroforestry that will soon be implemented. |
This is a complete re-write of emisCO2.R. The calculations of gross emissions (deforestation, degradation, other land conversion and harvesting emissions), are now fully aligned with the area-based emissions as one would calculate through tracking only the changes in land type areas multiplied by their carbon densities over time. The calculation for these "main emissions" is also improved, as it incorporates the "interaction emissions" which emerge when changes in land area and carbon densities influence one another. These are a relatively small source of emissions, roughly 0.1-1% compared to area emissions (land-use emissions). Aside from the inclusion of these, the main emissions haven't changed between implementations of emisCO2.R.
I have a series of plots generated from a suite of scenarios factorially switching forestry, degradation, CO2 afforestation, climate change, and SOM. Please find them attached to this PR, here: plots.zip. Of key importance for judging inconsistencies in the new approach will be the "residual emissions", between our total land use emissions (
emisArea
) and the sum of regrowth and gross emissions. The function now checks to ensure that these residual unaccounted for emissions are less that 1e-06. It also ensures that the stock derivation of carbonstock (which uses the raw carbon densities ov_carbon_stock) is roughly equal to our independent calculation. The check for this has to be cut off at 1e-05, because of one cell in REF. I'm eager to discuss further checks you might devise.@ Kristine, I mainly want you to check the implementation of the SOM module's CO2 emissions. Mostly, I've just separated the calculation into first generating the areas and then the carbon densities. You'll therefore want to in particularly attend to the composeDensity function. Notably, in my checks I found that pastures' SOM were not synchronized with the actual output of MAgPIE's carbonstock function. After Florian investigated further, we found that the implementation of SOM in the older emisCO2.R function was out of sync with the actual static SOM module implementation. In the actual implementation, the soil carbon in pasture is derived from
fm_carbon_density
. Therefore, I updated that here. Everything else should be the same. Since we don't have any mechanistic gross emission categories for SOM (like vegc has deforestation, e.g.), I'm attaching the "main emissions" for SOM to the "gross emissions" to make them match. You might also look at SOM emissions for a few choice scenarios (perhaps just SOM ON/SOM OFF) within the disaggregated emissions plots to make sure that they look reasonable. Something I'm interested to know, is whether you would expect the net SOM emissions to mostly be 0.You might also see some commented code relating to
croptreecover
, which is ongoing with Florian. I wanted to get this PR through before confronting those changes, so for now they will be comments.