-
Notifications
You must be signed in to change notification settings - Fork 126
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
Recipe and diagnostics to plot climate change hotspots: Cos et al., ESD 2022 #2614
Conversation
Very nice work @pepcos ! I am kind of jealous that you don't get to fight Codacy, not sure why the check does not appear... I will start reviewing but first, can you try to set a nice descriptive title for the pull request? |
Ok here we have the checks: https://app.codacy.com/gh/ESMValGroup/ESMValTool/pullRequest?prid=9121004 Try to address the easy ones first like thing related to missing docstrings, long lines, blank lines, and if you don't know how to solve them let me know commenting in the line. Then we can move on to the more "complicated" ones. |
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
The run got stuck in jasmin, but I am getting these warnings when collecting the provenance for the first diag:
I personally think it does not look very clean but go for it if you think it's better. |
Ok @pepcos , the recipe finished successfully by removing the dataset that is stored in a strange way. I have also added a fix for another dataset in , because it was crashing at the regrid step. As discussed offline, I removed the ensemble_statistics step as it was not impacting the results and it just added to the run time. Could you please try to take care of the warning in the provenance? |
Thanks @pepcos , the provenance warnings do not appear anymore. |
@pepcos I would say that from the technical point of view this looks fine to me, everything runs smoothly and other, than one codacy error that I think can be overlooked, the code looks fine. I will wait for the scientific results to be checked before giving the approve, in case something needs to be changed. |
…lGroup/ESMValTool into BSC_climate_change_hotspot
Hi everyone! Here is my take on the scientific review. From a scientific standpoint, the results obtained from the recipe seem fully consistent among them and in accordance with the current community knowledge as, for instance, regarding the warmer results obtained in CMIP6 when compared to CMIP5. On the other hand, the scenario combination plots offer a readily available tool to identify sound hotspot behaviours as well as variations within CMIP experiments, as it can be seen in the Central Australia domain. In summary, it seems the science parts are doing a good job on their expected function. Regarding the documentation, it is written in a clear and straightforward language, with examples that are easy to follow for any potentially interested user. |
Thanks, @rmarcos-ub. Excellent review! |
@pepcos Could you try merging the |
It seems only the documentation tests and codacy are run @bouweandela |
I think it may be because you did not link your account to CircleCI, in any case all tests are passing now. You can try to push again some small change (add a dot in the title?) or something and then remove it just for the sake of checking if you trigger the tests now. Or just leave it for another time, whatever you prefer. |
Description
This pull request aims to close the issue #2478 opened by @sloosvel
It contains the recipe and diagnostics to produce figures 2, 3, S1, S2 and S4 from Cos et al. 2022
Figure 2: Future Mediterranean climate change hotspot projected by CMIP5 and CMIP6 for a 8.5 Wm-2 radiative forcing
Figure 3: Scatter plot for the Mediterranean vs Global warming as projected by CMIP5 and 6 for three scenarios.
Figure S1: Same as Fig.2 but for 2.6 Wm-2 radiative forcing
Figure S2: Same as Fig.3 but for Mediterranean pr vs Global pr
Figure S4: Same as Fig.3 but for Mediterranean pr vs Global tas
Closes #2478
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.
- [ ] 🛠 Any changed dependencies have been added or removed correctlyNew or updated recipe/diagnostic
### New or updated data reformatting script- [ ] 🛠 Documentation is available- [ ] 🛠 The dataset has been added to the CMOR check recipe- [ ] 🧪 Numbers and units of the data look physically meaningfulTo help with the number of pull requests: