Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save resampled climates from KCS diagnostic local_resampling.py #2221

Merged
merged 9 commits into from
Feb 18, 2022

Conversation

Emmadd
Copy link
Contributor

@Emmadd Emmadd commented Jul 5, 2021

Description

Extends the KCS recipe to also output the actual resampled climates.
It's my first attempt at a pull request, I don't know how to check the automatic checks yet...

https://esmvaltool--2221.org.readthedocs.build/en/2221/recipes/recipe_kcs.html

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.

New or updated recipe/diagnostic


To help with the number of pull requests:

@Peter9192
Copy link
Contributor

Hey Emma,
Welcome and congratulations on your first PR! I'll try to find some time soon to do a review. One thing I noticed already is that you added daniels_emma as an author, but I don't think you added yourself to config_references as well, did you?

@Emmadd
Copy link
Contributor Author

Emmadd commented Jul 6, 2021

Hi Peter, I'm a bit overwhelmed with this whole process. Added it now.

@Peter9192 Peter9192 changed the title Output resampled climates as nc files from KCS diagnostic local_resampling.py Save resampled climates from KCS diagnostic local_resampling.py Jul 8, 2021
Copy link
Contributor

@Peter9192 Peter9192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Emmadd ,
Well done, this PR looks really neat! I have one small comment, but I'll already approve it.
Let me try and see if I can run the test bot, and maybe then someone from the scientific team (or Karin?) can verify that the results still look okay from a scientific point of view.

esmvaltool/config-references.yml Outdated Show resolved Hide resolved
@Peter9192
Copy link
Contributor

@esmvalbot please run recipe_kcs.yml

@esmvalbot
Copy link

esmvalbot bot commented Jul 8, 2021

Since @Peter9192 asked, ESMValBot will run recipe recipe_kcs.yml as soon as possible, output will be generated here

@Emmadd
Copy link
Contributor Author

Emmadd commented Jul 8, 2021

Hi, I haven't been able to see the generated output. Is there something I need to know? It looks like a cool feature to test recipes.

@Peter9192
Copy link
Contributor

Hi, I haven't been able to see the generated output. Is there something I need to know? It looks like a cool feature to test recipes.

The bot is really cool but today it's just very slow, or there is a queue, or something like that. It just hasn't run yet...

@esmvalbot
Copy link

esmvalbot bot commented Jul 9, 2021

ESMValBot is sorry to report it failed to run recipe recipe_kcs.yml: exit is 1, output has been generated here

@zklaus
Copy link
Contributor

zklaus commented Jul 12, 2021

The bot failed with a merge problem due to different types. This issue has cropped up before and is discussed in #2218 (comment) and links therein.

@Emmadd Emmadd added this to the v2.4.0 milestone Sep 2, 2021
@zklaus zklaus modified the milestones: [locked] v2.4.0, v2.5.0 Oct 27, 2021
@schlunma
Copy link
Contributor

schlunma commented Feb 9, 2022

@Emmadd Release v2.5 is approaching quickly (the feature freeze will be on 2022-02-21). Since this pull request is marked for v2.5, could you briefly comment if you further intend to include this in the new release and/or if you are facing any problems with it? Thanks!!

@schlunma schlunma self-requested a review February 16, 2022 09:05
Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just re-tested this recipe. It fails with a

ValueError: 'year' not present in all datasets and coords='different'. Either add 'year' to datasets where it is missing or specify coords='minimal'.

log.txt

EDIT: The same error appears in the original recipe, so this probably not related to this change:

https://esmvaltool.dkrz.de/shared/esmvaltool/v2.5.0-test-rc2/recipe_kcs_20220213_153930/run/global_matching/global_matching/log.txt

@schlunma schlunma mentioned this pull request Feb 18, 2022
11 tasks
Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested this again. The recipe runs fine now, and all output a looks as expected! 馃帀 Thanks @Emmadd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants