Scm Run#47
Conversation
|
It seems to work fine but I still have to check it with a REMIND output. I leave here some considerations/questions:
|
Ok great. Once you've fixed the merge conflicts I'll take a look. Maybe there are some simplifications we can make, but sounds like a good start.
Yep: this is sufficient. I would use the quantiles plus the peak warming and 2100 warming metadata (which is technically a test of post-processing, but ok, they're the key numbers that people really care about so that's what we should test).
I'll take a look once you've fixed the merge conflicts. There are probably some tricks I played in setting things up in |
|
Hmmm is the regression output you're comparing against from the gridding or global workflow? If you compare the key metrics e.g. peak warming, are you close or miles away there too? Also are we comparing post-processed temperatures or raw temperatures? |
|
Ok solved. I wasn't taking care of some
|
There are smarter ways but for ease, just add it.
Nope: only check against the key metrics i.e. peak warming and quantiles. Then we don't have to save the ensemble members anywhere |
Some filer are a bit large though. E.g. |
|
Ok I got it all green here: mzecc#6 I did some ci tweaking for |
Yep this isn't ideal, but we have to make a tradeoff. Just commit with
Ok good idea, but in my experience lfs is a trap. When we try and pull this into main (which may be soon or now?), we will do this differently and not use lfs at all (including making sure there are no lfs files in history). Delete |
|
All green here, I'll add the changelog and that's it I guess. |
|
Once #43 is merged, can you please squash and rebase onto main (I want to squash and rebase in this case so we're sure we didn't add any git lfs stuff by mistake). |
|
Once you've squashed and rebased, please add the CHANGELOG entries, then I'll review properly and then we can hopefully merge |
mzecc
force-pushed
the
scm_running
branch
2 times, most recently
from
8dd6fc0 to
a6e978d
Compare
|
Ok, this looks a bit more clean now I think. |
znichollscr
left a comment
znichollscr
left a comment
There was a problem hiding this comment.
Nice, few things to clean up then good to go I think
| complete_emissions.columns = complete_emissions.columns.astype(int) | ||
| magicc_start_year = 2015 | ||
| if min(complete_emissions.columns) != magicc_start_year: | ||
| msg = "Emissions starting year must be set to `2015`" | ||
| raise AssertionError(msg) | ||
| else: | ||
| complete_emissions = get_complete_scenarios_for_magicc( | ||
| scenarios=in_emissions, | ||
| history=self.historical_emissions, | ||
| ) |
There was a problem hiding this comment.
Is something like this acceptable?
I mean, should we allow for a unique dataframe with history and emissions already prepared to run through?
There was a problem hiding this comment.
Hmm it still doesn't really explain what is going on. Like this code just says, "If there's no historical emissions, you have to start in 2015, otherwise I will join history and the scenarios", but what we actually want is, "If you're running MAGICC and there's no historical emissions, you have to start in 2015, otherwise if you're running MAGICC, I will join history and the scenarios, otherwise (if you're running some other SCM) I'll just try to use the emissions as given".
If it's too much headache, just leave it so you can get to the runs you need to do and we can clean up in future.
|
Ok, all green. Ready to be merged away? |
|
Let's go for it |
2 participants
Description
Added class for MAGICC run.
Checklist
Please confirm that this pull request has done the following:
changelog/