OMS-NetZero / FAIR

Finite-amplitude Impulse Response simple climate model
https://docs.fairmodel.net
Apache License 2.0
123 stars 62 forks source link

Fix CO2 concentrations and radiative forcing after restart_in #52

Closed chrisroadmap closed 5 years ago

chrisroadmap commented 5 years ago

Pull request

Please confirm that this pull request has done the following:

Only a minor fix so not gone through the usual protocols, but existing tests pass.

restart_out and restart_in have been around since v1.0, but are probably not often used. They are useful for experiments where the user wants to switch from emissions driven to concentration- or forcing-driven in the same run. restart_in allows a FaIR run to use a spun-up carbon cycle and thermal response from an existing run.

The problem occurred when using restart_in in CO2-only mode. In the first time step the model was not picking up the atmospheric concentrations of CO2 and radiative forcing from CO2 calculated from restart_in. This has now been fixed.

Some re-factoring is earmarked for the restart_in / restart_out functionality, and it probably needs extending to non-CO2 drivers.

Proposing to release this patch as v1.3.6 as an upcoming paper relies on this working for reproducible results.

codecov[bot] commented 5 years ago

Codecov Report

Merging #52 into master will increase coverage by 0.31%. The diff coverage is 91.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   90.47%   90.78%   +0.31%     
==========================================
  Files          33       33              
  Lines        1260     1270      +10     
==========================================
+ Hits         1140     1153      +13     
+ Misses        120      117       -3
Impacted Files Coverage Δ
fair/forward.py 83.7% <91.89%> (+1.78%) :arrow_up:

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 4489f92...b5a8278. Read the comment docs.

chrisroadmap commented 5 years ago

I think actually this needs to be part of a bigger refactoring exercise because restart behaves differently with CO2 only and multi-gas. Hold off approval for now @znicholls

chrisroadmap commented 5 years ago

OK, ready to go.

znicholls commented 5 years ago

Only other comment would be that at some point, we may want to think about automating the generation of the example png's so they don't need to be in the repo. As they're so small at the moment, not an issue so leave it for a rainy day when you're bored.