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.

Sign up for GitHub

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

Jump to bottom

New recipe and diagnostic for Arctic-midlatitude research #3021

Merged
merged 106 commits into from
Jun 19, 2023
Merged

New recipe and diagnostic for Arctic-midlatitude research #3021

merged 106 commits into from
Jun 19, 2023

Conversation

egalytska
Copy link
Contributor

@egalytska egalytska commented Jan 19, 2023 •
edited
Loading

Description

This pull request provides a recipe and diagnostic to calculate variables that are further used by causal discovery. Selected variables represent major drivers of Arctic-midlatitude teleconnections. This will be a part of the Galytska et al manuscript.

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.

New or updated recipe/diagnostic

egalytska and others added 16 commits September 9, 2022 12:25
@remi-kazeroni remi-kazeroni added this to the v2.8.0 milestone Jan 19, 2023
@bsolino
Copy link
Contributor

bsolino commented Feb 13, 2023

Hi @egalytska!,
I've just started with the PR. I have noticed that the tests are failing because of some issues with the code standard. I have solved some of them automatically (using these commands), but there are still a few that can't be resolved that way:
./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:5:1: F401 'numpy as np' imported but unused ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:14:1: F401 'esmvaltool.diag_scripts.shared.save_data' imported but unused ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:73:18: N806 variable 'psl_Ural' in function should be lowercase ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:76:18: N806 variable 'psl_Sib' in function should be lowercase ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:79:18: N806 variable 'psl_Aleut' in function should be lowercase ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:85:18: N806 variable 'sic_BK' in function should be lowercase ./esmvaltool/diag_scripts/galytska22/select_actors_for_tigramite.py:88:18: N806 variable 'sic_Ok' in function should be lowercase

You can find this information this below in the "ci/circleci: run_tests" and the "Codacy Static Code Analysis" details.

bsolino
bsolino reviewed Feb 13, 2023
View reviewed changes
@egalytska egalytska marked this pull request as ready for review June 15, 2023 13:44
@remi-kazeroni remi-kazeroni removed the requires new ESMValCore release A new release of ESMValCore is needed to solve this issue/merge this pull request.. label Jun 16, 2023
remi-kazeroni
remi-kazeroni reviewed Jun 16, 2023
View reviewed changes
Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all the previous points, @egalytska! I have 2 minor suggestions for you in order to get all green lights and proceed with the rest of the technical review :+1

@egalytska
Copy link
Contributor Author

Hi @remi-kazeroni, thanks for the comments.
I think I have addressed all the issues.

remi-kazeroni
remi-kazeroni approved these changes Jun 16, 2023
View reviewed changes
Copy link
Contributor

@remi-kazeroni remi-kazeroni left a comment

Choose a reason for hiding this comment

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

@egalytska - Thanks a lot for your contribution and addressing the comments! All looks good to me from a technical point of view. The recipe runs fine and produces provenance records for the nc files. As the recipe needs to be run on a full compute node, I added an entry to generate.py.

@LisaBock - could you please have a final look for the scientific review, possibly on Monday? This way the PR could still make it in before feature freeze for v2.9 👍
If it helps, here is a path to a successful run: /work/bd0854/b309192/runs/tests/PR_3021/esmvaltool_output/recipe_galytska23jgr_20230616_133228

@egalytska
Update esmvaltool/diag_scripts/galytska23/select_variables_for_tigram…
208b029 
…ite.py


add referense

Co-authored-by: Rémi Kazeroni <remi.kazeroni@dlr.de>
LisaBock
LisaBock approved these changes Jun 19, 2023
View reviewed changes
Copy link
Contributor

@LisaBock LisaBock left a comment

Choose a reason for hiding this comment

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

Thanks @egalytska for your work! Looks like all my comments are addressed.

@schlunma
Copy link
Contributor

Thanks all 🚀

@schlunma schlunma merged commit a9c8a26 into main Jun 19, 2023
@schlunma schlunma deleted the Arctic-midlat_EG branch June 19, 2023 09:08
ehogan added a commit that referenced this pull request Jun 26, 2023
@ehogan
Merge branch 'recipe_test_workflow_prototype' into separate_install_c…
d8efedf 
…old_and_clone_task_rtw

* recipe_test_workflow_prototype: (199 commits)
  #3169: Upgrade the RTW to work with ESMValTool v2.8.0
  [Condalock] Update Linux condalock file (#3237)
  Modified links to the tutorial (#3236)
  Add ESMValCore release `v2.8.1` into the documentation (#3235)
  Generate climatology on the fly for AutoAssess soil moisture (#3197)
  New recipe and diagnostic for Arctic-midlatitude research (#3021)
  Fixed pandas diagnostics for pandas>=2.0.0 (#3209)
  Update obs4MIPs dataset to the current naming scheme in recipe_smpi.yml (#2991)
  Add variable long names to provenance record in monitoring diagnostics (#3222)
  Extension of NASA MERRA2 CMORizer (cl, cli, clivi, clw, clwvi) (#3167)
  Remove "fx_variable" from recipe_wenzel14jgr.yml (#3212)
  [Condalock] Update Linux condalock file (#3217)
  Add Seaborn diagnostic (#3155)
  Remove fx_variables from ipccwg1ar5ch9 recipes (#3215)
  Remove "fx_variable" from recipe_tebaldi21esd.yml (#3211)
  Update recipe_impact.yml to work with newer versions of `pandas` (#3220)
  Use ESMValCore v2.9.0 release candidates (#3219)
  [Github Actions ] Check if python minor version changed after Julia install in development installation test (#3213)
  New plot_type 1d_profile in monitor  (#3178)
  Add support for using a dask distributed scheduler (#3151)
  ...
@hb326 hb326 mentioned this pull request Jul 3, 2023
Merged
6 tasks
jvegreg pushed a commit that referenced this pull request Jan 14, 2024
@egalytska @bsolino @LisaBock
New recipe and diagnostic for Arctic-midlatitude research (#3021)
121cff0 
Co-authored-by: Evgenia Galytska <b380971@levante3.atos.local>
Co-authored-by: Evgenia Galytska <b380971@levante5.atos.local>
Co-authored-by: Evgenia Galytska <b380971@levante4.atos.local>
Co-authored-by: Brei Soliño <b.solinofernandez@gmail.com>
Co-authored-by: Breixo Soliño <Breixo.SolinoFernandez@dlr.de>
Co-authored-by: Rémi Kazeroni <remi.kazeroni@dlr.de>
Co-authored-by: Lisa Bock <lisa.bock@dlr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsolino bsolino bsolino left review comments

@LisaBock LisaBock LisaBock approved these changes

@remi-kazeroni remi-kazeroni remi-kazeroni approved these changes

Assignees
No one assigned
Labels
approved by scientific reviewer approved by technical reviewer diagnostic
Projects
None yet
Milestone
v2.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@egalytska @bsolino @remi-kazeroni @schlunma @LisaBock