OMS-NetZero / FAIR

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

Restarts for multigas and GIR carbon #94

Closed SallyDa closed 3 years ago

SallyDa commented 3 years ago

Hello FaIR developers,

First of all I just want to say that I think this model is a really nice idea!

I recently started working with @Hans-PeterH and @myrti, looking at companies contributions to global warming. We are using FaIR in a web app and want to run it multiple times on the fly, so we are trying to save computational time as much as possible. As we are only changing future emissions and the historical part remains the same, restarts from present day seem like a good option.

This pr adds restart functionality for multi-gas mode and the GIR carbon cycle. Additionally I found and fixed a bug when trying to use CO2-only mode with the GIR carbon cycle (line 752 old, 826 new of fair/forward.py).

Finally, I am sorry that I added even more lines of code and more if else statements to an already very long function. I did think briefly about trying to clean up the code a bit, but I know you are already working on that. I am so excited for v2!! In the meantime though, it would be great if we could get this pr merged and a new v1 release that we can use for our tool.

Thanks, Sally

codecov[bot] commented 3 years ago

Codecov Report

Merging #94 (551d9e9) into master (31c4ae0) will increase coverage by 0.46%. The diff coverage is 96.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   95.07%   95.54%   +0.46%     
==========================================
  Files          42       42              
  Lines        1787     1818      +31     
==========================================
+ Hits         1699     1737      +38     
+ Misses         88       81       -7     
Impacted Files Coverage Δ
fair/forward.py 84.73% <96.36%> (+2.87%) :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 31c4ae0...551d9e9. Read the comment docs.

SallyDa commented 3 years ago

Hello,

This pull request has been here for a while now. It would be nice to get an update from someone (@chrisroadmap?) to find out whether this is something you are interested in merging at some point and if so, when you would have time to look at it. If there's anything else you would like me to do, please just let me know.

Sally

myrti commented 3 years ago

Hello,

To reiterate Sally's request: Could you please let us know your verdict on this change. Using the restart option leads to a significant speed increase for running different future scenarios which makes it valuable to us.

Thank you!

chrisroadmap commented 3 years ago

Sorry, I have a lot on my plate at the minute... I will check this as soon as I get a chance!

chrisroadmap commented 3 years ago

Hey @SallyDa, could you rebase the latest master branch on to this pull request? I think it will then be straightforward to merge it.

chrisroadmap commented 3 years ago

I'll check what needs to be done with #95 and roll this into a v1.6.3 release later today. Sorry I'm so slow with all of this!

SallyDa commented 3 years ago

Thanks a lot @chrisroadmap, this is really great news for us!!