add oxidised methane to total CO2, rather than the atmospheric boxes

[chrisroadmap]

Pull request

addresses #93

• Tests added
• Documentation added (where applicable) N/A as fix
• Example added (either to an existing notebook or as a new notebook, where applicable)
• Description in CHANGELOG.rst added

[rgieseke]

Interestingly this doesn't fail any of the pre-calculated tests: so while it was implemented clunkily it wasn't affecting the model results.

Could this be because fossilCH4_frac=0. leading to (molwt.C/molwt.CH4 * 0.001 * oxCH4_frac * fossilCH4_frac[t])) being zero with this default?

[codecov]

Codecov Report

Merging #95 (dd53813) into master (916265b) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #95   +/-   ##
=======================================
  Coverage   95.41%   95.41%           
=======================================
  Files          41       41           
  Lines        1744     1744           
=======================================
  Hits         1664     1664           
  Misses         80       80           

Impacted Files	Coverage Δ
fair/forward.py	82.20% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 916265b...dd53813. Read the comment docs.

[FrankErrickson]

Interestingly this doesn't fail any of the pre-calculated tests: so while it was implemented clunkily it wasn't affecting the model results.

Could this be because fossilCH4_frac=0. leading to (molwt.C/molwt.CH4 * 0.001 * oxCH4_frac * fossilCH4_frac[t])) being zero with this default?

This is actually how we first came across the oxidised CO2 issue. Tests comparing the Julia and Python results wouldn't pass when fossilCH4_frac was anything but the default 0.

[znicholls]

Reading the comments, is it worth adding a test with the oxidisation fraction being non-zero?

[chrisroadmap]

Yep, leave it with me. I think the default is zero because we need some more scenario information on the CH4 fossil to total split which not all scenarios give. I remember the effect being quite small, will do some diagnostics.

[chrisroadmap]

So to summarise, this was an error, and yes 4x the intended amount of methane was being added to the atmospheric CO2. The impacts were small, which is probably why it went un-noticed, and you had to explicitly set a non-zero methane fraction in order to get this issue occurring.

[FrankErrickson]

@chrisroadmap Just wanted to double check, do the units also need to be adjusted now that the oxidised CH4 is being added to CO2 emissions rather than the carbon pools? The terms molwt.C/molwt.CH4 * 0.001 appear in the current oxidised CH4 equation code, but not in Equation 6 of the FAIR v1.3 model paper (where it's described as additional CO2 emissions).

[FrankErrickson]

Also in the carbon_cycle function, are oxidised CH4 emissions now being added to the period t-1, but not the period t emissions?

[chrisroadmap]

@chrisroadmap Just wanted to double check, do the units also need to be adjusted now that the oxidised CH4 is being added to CO2 emissions rather than the carbon pools?

Yes, I think so.

Also in the carbon_cycle function, are oxidised CH4 emissions now being added to the period t-1, but not the period t emissions?

The loop runs from 1, so it picks up emissions starting from year 0.

[chrisroadmap]

As you can see it's a bit of a second order effect and causing a few headaches - we'll either reform or remove it in fair 2.0

[chrisroadmap]

hmm, this doesn't seem to play nice with the recently added restart branch. One for the future.

[znicholls]

hmm, this doesn't seem to play nice with the recently added restart branch. One for the future.

too late now but I probably would have done this before the restarts...

[chrisroadmap]

Will close the PR now because methane oxidation is one of those hangover issues - but it will be addressed in a future v2.x