-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AR6 Climate assessment on reporting #1475
Conversation
There are still two big pain points that should probably be addressed before this is merged. Could someone familiar with Python and our
|
Hi Gabriel, I don't really know much about python venvs, Mika was our expert here. However, it seems like you are using run_harm_inf.py and run_clim.py which are both quite trivial scripts. They |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, Gabriel, thanks…
My first set of comments ;)
I have a preference for not calling it "ar6climate", because in AR6 different climate models were used, but rather MAGICC7.5.3 or something like that.
Concerning this line You should be able get this number by acessing "SLURM_NTASKS" on the cluster. A failsafe solution could look like this: There might be other useful environment variables available. Please check out this tutorial: https://www.pik-potsdam.de/en/institute/about/it-services/hpc/user-guides/slurm
But of course, if this is the case, then SLURM_NTASKS will always be 1. |
@gabriel-abrahao: Merge this to improve slurm options in output.R: gabriel-abrahao#1 |
allow 12 tasks slurm option for MAGICC7.5.3 in output.R
Co-authored-by: orichters <90761609+orichters@users.noreply.github.com>
Revised ar6Climate reporting script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noted some minor things that have changed since Gabriel started working on it and that should be kept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I created a new merge conflict, I have fixed the stuff that I noted.
Some results for
|
Comparison for NGFS3, so 2022, where I have both MAGICC6 from REMIND and MAGICC7.5.3 as run by IIASA at hand:
click to expand code!
|
I did another test using NGFS4, so 2023, data. I took a REMIND NGFS run from 2023, ran the ar6climate reporting script, and compare it to the AR6 assessment I have received from IIASA:
There is some slight variation that I don't understand. But overall looks good. click to expand code!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is fine from my side. Still might be worth investigating why the results slightly differ, but this is a big step forward, thank you @tonnrueter and @gabriel-abrahao.
@LaviniaBaumstark: Can you have a look as well, please, mostly whether you consider the last table good enough in terms of results.
I would see no reason for not merging. Differences in REMIND_MAGICC6 vs. REMIND_MAGICC7.5.3 is no suprice. Why REMIND_MAGICC7.5.3 vs. IIASA_MAGICC7.5.3 are not the same is maybe related to some inconcistend parameterisation but nothing we need to clarify before merging. We are clode enough. |
but: please run make test |
Please run |
I agree with Lavinia here, the differences in Oli's tests don't seem large enough to justify not merging. I've put a lot more work into making sure everything is correct in this version here than in the previous one, so if there's an actual mistake, it's more likely that the differences came from fixing it. That said, a lot of segfaults and overflows usually happen in some individual MAGICC parametrizations. The IIASA people told me they are aware of it but didn't really give me an explanation yet, so maybe it's related to that. At the end of the day, that's the official AR6 pipeline, so if there's a problem, everyone else also has it, and they are aware. |
Thanks y'all for chiming in! I'll run the tests and merge asap |
Purpose of this PR
This PR adds the option to run AR6's climate-assessment as part of the normal
output.R
procedure, under the optionar6Climate
. If one wants it to be run automatically, it can be added tocfg$output
. This includes the probabilistic assessment of temperature change, compatible with the AR6 standards.By default, the whole 600 member MAGICC v7.5.3 ensemble is run, which can take a while. Only certain percentiles are output, with each percentile in a different variable. Although the original output is yearly, which can be found inside a subfolder
climate-temp
, only data for the REMIND timesteps are added to the standard MIF (REMIND_generic***
). All new variables have theAR6 climate diagnostics|
prefix. The original temperature variable,Temperature|Global Mean
, is still calculated with the old (v6) MAGICC. The comparable variable in the new assessment isAR6 climate diagnostics|Surface Temperature (GSAT)|MAGICCv7.5.3|XXth percentile
, with separate variables for 10 different percentiles.The subfolder
climate-temp
includes the full output ofclimate-assessment
, including files describing temperature exceedence probabilities and the consequent climate classification of the scenario (C1, C2, etc).Type of change
Checklist:
remind2
where it was neededforbiddenColumnNames
in readCheckScenarioConfig.R in case the PR leads to deprecated switchesFAIL 0
in the output ofmake test
)CHANGELOG.md
has been updated correctly