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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for using a dask distributed scheduler #3151

Merged
merged 12 commits into from
Jun 6, 2023

Conversation

bouweandela
Copy link
Member

@bouweandela bouweandela commented Apr 4, 2023

Description

Configure the Dask distributed scheduler so diagnostics can make use of it.

To be used with ESMValGroup/ESMValCore#2049.

Link to documentation: https://esmvaltool--2049.org.readthedocs.build/projects/ESMValCore/en/2049/quickstart/configure.html#dask-distributed-configuration


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.


To help with the number of pull requests:

@bouweandela bouweandela marked this pull request as ready for review May 23, 2023 13:21
@bouweandela bouweandela added this to the v2.9.0 milestone May 23, 2023
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 tested this together with ESMValGroup/ESMValCore#2049 and it works really well!! 🚀

One very minor comment below.

Comment on lines 572 to 578
except OSError:
logger.error(
"Unable to connect to the Dask distributed scheduler at %s. "
"If the scheduler is no longer available, try re-running the "
"diagnostic script with the --no-distributed flag.",
cfg['scheduler_address'])
raise
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it nicer to include the error message in an exception than calling logger.error, i.e., something like raise Exception("...") from ... where Exception should be something sensible. In my experience, users only look at the exceptions and tend to overlook error messages further up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in cfd97aa

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

grammarnazi is back

esmvaltool/diag_scripts/shared/_base.py Outdated Show resolved Hide resolved
@valeriupredoi
Copy link
Contributor

I'll commit the wee grammar thingie, and merge, thanks a lot for your good work Bouwe and Manu! Alas, I thought we wanted to add a unit test in test_base.py too, but maybe we can do that once the docs and all else is in place?

Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
@bouweandela
Copy link
Member Author

Do you reckon we need additional documentation? If yes, where should we put it?

@valeriupredoi
Copy link
Contributor

valeriupredoi commented Jun 6, 2023

Do you reckon we need additional documentation? If yes, where should we put it?

yes a wee pointer in ESMValTool docs pointing to the Core docs on how to do dasking. It's Dask, man, it's like Cindy Crawford and Friends in the 90s, everyone talks about it, so something a la "Dask enabled magic" has to be in the ESMValTool docs too

@valeriupredoi valeriupredoi merged commit ab4c1da into main Jun 6, 2023
@valeriupredoi valeriupredoi deleted the dask-distributed branch June 6, 2023 14:44
ehogan added a commit that referenced this pull request Jun 26, 2023
…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)
  ...
jvegreg pushed a commit that referenced this pull request Jan 14, 2024
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
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.

3 participants