OMS-NetZero / FAIR

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

Separate carbon cycle #60

Closed chrisroadmap closed 5 years ago

chrisroadmap commented 5 years ago

Pull request

Please confirm that this pull request has done the following:

Does not require example as does not affect underlying model operation or interface

codecov[bot] commented 5 years ago

Codecov Report

Merging #60 into master will increase coverage by 0.22%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
+ Coverage    91.9%   92.12%   +0.22%     
==========================================
  Files          33       33              
  Lines        1272     1270       -2     
==========================================
+ Hits         1169     1170       +1     
+ Misses        103      100       -3
Impacted Files Coverage Δ
fair/forward.py 84.87% <100%> (+0.98%) :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 7cb0a14...ff4d815. Read the comment docs.

chrisroadmap commented 5 years ago

Some of the parameters passed to carbon_cycle are the same as in fair_scm - there are about 15 arguments which is a little unwieldy as it stands and perhaps they can just be made global. On the other hand the benefit of including everything is that carbon_cycle can be run stand-alone.

znicholls commented 5 years ago

perhaps they can just be made global

don't do that, global arguments will make your life a nightmare

a little unwieldy as it stands

I think it's going to have to be that way for a little bit so you can make it easy to then later refactor into objects. Think of it as the storm before the calm and trust the tests to ensure that nothing fatal changes.