OMS-NetZero / FAIR

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

add SSP module interface #101

Closed chrisroadmap closed 2 years ago

chrisroadmap commented 2 years ago

Addresses #43

chrisroadmap commented 2 years ago

wait for #102 (a good test would be if this branch fails coverage without tests being added)

codecov[bot] commented 2 years ago

Codecov Report

Merging #101 (f5b884f) into master (c7bf674) will decrease coverage by 0.36%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
- Coverage   95.54%   95.17%   -0.37%     
==========================================
  Files          42       40       -2     
  Lines        1818     1679     -139     
==========================================
- Hits         1737     1598     -139     
  Misses         81       81              
Impacted Files Coverage Δ
fair/RCPs/__init__.py 100.00% <100.00%> (ø)
fair/RCPs/_shared.py 100.00% <100.00%> (ø)
fair/SSPs/__init__.py 100.00% <100.00%> (ø)
fair/SSPs/_shared.py 100.00% <100.00%> (ø)
fair/forcing/aerosols.py 92.72% <100.00%> (+0.13%) :arrow_up:
fair/tools/magicc.py 100.00% <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 5a3b7e7...f5b884f. Read the comment docs.

chrisroadmap commented 2 years ago

I shouldn't be happy that coverage has gone down, but I am here, because it means the CI is working

znicholls commented 2 years ago

Not sure if this still a draft, but I'm hoping that you're planning on removing all the duplicate code? It must be possible to get the same interfaces without all this duplication (tidying up the RCPs at the same time is probably not a bad idea either).

chrisroadmap commented 2 years ago

Any ideas how to do this? I know what I'm doing currently is very unpythonic but it's been around a while.

Just want this to work:

from fair.SSPs import rcp119
from fair.forward import fair_scm

...

fair_scm(emissions=ssp119.Emissions.emissions)

What I need to figure out is how to put all of the duplicated stuff in one module, import the particular SSP, then get the SSP module to communicate with the shared module (give it the specific SSP to look up from the emissions data file, then put the results in the Emissions class) and be available to the main interface using the original SSP name.

znicholls commented 2 years ago

How about something like

# shared module
class Emissions:
    def __init__(self, emissions):
        self.emissions = emissions
        # etc. etc. 

def load_emms_data(fp):
    # do the loading, return something which has the attributes you want
    # e.g.
    loaded = pd.read_csv(fp)
    out = Emissions(loaded)

    return out
# SSPs module
class ssp119:
    Emissions = load_emms_data("path_to_ssp119.txt")

# then you should be able to do
ssp119.Emissions.emissions  # etc.
chrisroadmap commented 2 years ago

@znicholls OK let me know what you think. I could probably avoid duplication in the __init__.py files but can't figure out exactly how to do it. Coverage decreased because we removed code! (this won't stop me merging as still over 95%).

znicholls commented 2 years ago

Coverage decreased because we removed code! (this won't stop me merging as still over 95%).

Nice, only one suggestion in the comment above