Fixing CI #929

mmccarty · 2022-05-20T20:13:51Z

This PR follows on from #909 and fixes the CI build with following changes

adds :okwarning: and :okexcept: options to sphinx ipython blocks
~~pin versions of sklearn and scipy that are currently compatible with dask-ml~~
upgrade test and associated code to support both sklearn 1.0 and 1.1
removes xgboost from ci/environment-docs.yml

If this PR is acceptable, #924 and #931 can be closed.

This reverts commit 54771f3.

mmccarty · 2022-05-24T17:11:40Z

cc @jrbourbeau

mmccarty · 2022-05-25T00:35:48Z

Need to add checks to swtich between scikit-learn 1.0.* and 1.1.

dask_ml/metrics/regression.py

dask_ml/_compat.py

wence-

Minor quibble

ci/environment-3.8.yaml

docs/source/conf.py

Co-authored-by: James Bourbeau <jrbourbeau@users.noreply.github.com>

VibhuJawa · 2022-05-25T18:17:43Z

docs/source/hyper-parameter-search.rst

@@ -420,6 +420,7 @@ This section uses :class:`~dask_ml.model_selection.HyperbandSearchCV`, but it ca
 also be applied to to :class:`~dask_ml.model_selection.IncrementalSearchCV` too.

 .. ipython:: python
+   :okwarning:


We should open issues corresponding to the problems that we are ignoring by this. Essentially to ensure that we have it in the backlog.

informative_idx, beta = dask.compute( informative_idx, beta, scheduler="single-threaded" )

💯 agreed. @VibhuJawa - Do you have a reproducer? I wasn't able to get one outside of sphinx ipython directives.

import dask.distributed # sets dask.config["scheduler"] == "dask.distributed" import dask.array as da import dask a = dask.compute(da.arange(10), scheduler="single-threaded")

make_classification explicitly calls (as @VibhuJawa indicates) dask.compute setting the scheduler to "single-threaded", which then mismatches with the scheduler set up by importing dask.distributed.

Plausibly this is the correct fix

diff --git a/dask_ml/datasets.py b/dask_ml/datasets.py index a561ee0d..3a5f5e99 100644 --- a/dask_ml/datasets.py +++ b/dask_ml/datasets.py @@ -370,9 +370,7 @@ def make_classification( informative_idx = rng.choice(n_features, n_informative, chunks=n_informative) beta = (rng.random(n_features, chunks=n_features) - 1) * scale - informative_idx, beta = dask.compute( - informative_idx, beta, scheduler="single-threaded" - ) + informative_idx, beta = dask.compute(informative_idx, beta) z0 = X[:, informative_idx].dot(beta[informative_idx]) y = rng.random(z0.shape, chunks=chunks[0]) < 1 / (1 + da.exp(-z0))

Although that backs out cb5c2ee which looks like it was a deliberate choice at the time.

@mmccarty, Minimal Example

from dask.distributed import Client import dask_ml.datasets client = Client() X,y = dask_ml.datasets.make_classification_df(chunks=100)

/datasets/vjawa/miniconda3/envs/dask-ml-dev/lib/python3.8/site-packages/dask/base.py:1282: UserWarning: Running on a single-machine scheduler when a distributed client is active might lead to unexpected results. warnings.warn(

Please note that there is a call in k-means that will need to be eventually fixed too.

dask-ml/dask_ml/cluster/k_means.py

Lines 472 to 475 in 30b3dea

rng2 = (

random_state.randint(0, np.iinfo("i4").max - 1, chunks=())

.compute(scheduler="single-threaded")

.item()

@wence- , I tried fixing it by removing the explicit cals with #924 but that causes issues with key-names downstream here . I suggest going with @mmccarty's fix to unblock the CI and then we can tackle this issue from the backlog.

@VibhuJawa - Ah, of course, the warning is reproducible. Sorry, I was thinking about the key error. Opened an issue #933

VibhuJawa · 2022-05-25T18:17:43Z

docs/source/hyper-parameter-search.rst

@@ -420,6 +420,7 @@ This section uses :class:`~dask_ml.model_selection.HyperbandSearchCV`, but it ca
 also be applied to to :class:`~dask_ml.model_selection.IncrementalSearchCV` too.

 .. ipython:: python
+   :okwarning:


We should open issues corresponding to the problems that we are ignoring by this. Essentially to ensure that we have it in the backlog.

informative_idx, beta = dask.compute( informative_idx, beta, scheduler="single-threaded" )

VibhuJawa · 2022-05-25T18:18:01Z

docs/source/meta-estimators.rst

@@ -56,6 +56,7 @@ This class is useful for predicting for or transforming large datasets.
 We'll make a larger dask array ``X_big`` with 10,000 samples per block.

 .. ipython:: python
+   :okwarning:


Same for this.

VibhuJawa · 2022-05-25T18:18:01Z

docs/source/meta-estimators.rst

@@ -56,6 +56,7 @@ This class is useful for predicting for or transforming large datasets.
 We'll make a larger dask array ``X_big`` with 10,000 samples per block.

 .. ipython:: python
+   :okwarning:


Same for this.

wence- · 2022-05-26T10:06:49Z

dask_ml/_compat.py

@@ -29,6 +29,14 @@
 PANDAS_1_2_0 = PANDAS_VERSION > packaging.version.parse("1.2.0")
 WINDOWS = os.name == "nt"

+SKLEARN_1_1_X = SK_VERSION >= packaging.version.parse("1.1")


This name is misleading, since it is True for 1.2 (say). I realise this mimics the naming above, so I guess fine.

This is the convention used by packages like six.

wence-

Modulo minor comments, I think this (from my inexpert eye) looks fine.

TomAugspurger · 2022-05-27T14:00:15Z

dask_ml/_compat.py

@@ -29,6 +29,14 @@
 PANDAS_1_2_0 = PANDAS_VERSION > packaging.version.parse("1.2.0")
 WINDOWS = os.name == "nt"

+SKLEARN_1_1_X = SK_VERSION >= packaging.version.parse("1.1")


This is the convention used by packages like six.

TomAugspurger · 2022-05-27T14:02:53Z

Thanks! Just to confirm, we have issues for each of the distinct warnings we're now silencing?

mmccarty · 2022-05-27T14:13:39Z

Thanks @TomAugspurger! We have one issue #933 to follow up, which I'll look at soon. It may lead to more issues but we can track them there.

stsievert added a commit to kchare/dask-ml that referenced this pull request May 21, 2022

manually merge dask#929

54771f3

stsievert added a commit to kchare/dask-ml that referenced this pull request May 21, 2022

Revert "manually merge dask#929"

d7c57d4

This reverts commit 54771f3.

mmccarty marked this pull request as ready for review May 23, 2022 14:43

mmccarty mentioned this pull request May 23, 2022

Roll a new release #928

Closed

jakirkham requested a review from TomAugspurger May 23, 2022 19:31

mmccarty added 4 commits May 24, 2022 19:12

fixing build

4aea349

docs fixes

8bb9a78

added new lines at the end of environment.yaml files

0272772

upgrade to sklearn==1.1

7414062

mmccarty force-pushed the ci-fixes branch from e25f8e4 to 7414062 Compare May 24, 2022 23:18

enabled kmeans dataframe test

c7fd86b

switches for sklearn 1.0.8 & 1.1

8df8085

mmccarty closed this May 25, 2022

mmccarty reopened this May 25, 2022

charlesbluca reviewed May 25, 2022

View reviewed changes

dask_ml/metrics/regression.py Show resolved Hide resolved

dask_ml/_compat.py Outdated Show resolved Hide resolved

wence- reviewed May 25, 2022

View reviewed changes

ci/environment-3.8.yaml Outdated Show resolved Hide resolved

jrbourbeau reviewed May 25, 2022

View reviewed changes

docs/source/conf.py Outdated Show resolved Hide resolved

mmccarty and others added 4 commits May 25, 2022 10:26

Update docs/source/conf.py

ac7ed7b

Co-authored-by: James Bourbeau <jrbourbeau@users.noreply.github.com>

addressing review comments

1612e07

minor grammar fix

45e4514

enabled grid search precomputed kernel test

05168b6

VibhuJawa reviewed May 25, 2022

View reviewed changes

mmccarty requested review from wence-, charlesbluca and jrbourbeau May 25, 2022 19:19

wence- reviewed May 26, 2022

View reviewed changes

wence- approved these changes May 26, 2022

View reviewed changes

TomAugspurger approved these changes May 27, 2022

View reviewed changes

TomAugspurger merged commit 7b8c8e0 into dask:main May 27, 2022

mmccarty mentioned this pull request May 27, 2022

Unexpected UserWarning: Single-machine scheduler when a distributed client is active might lead to unexpected results #933

Open

mmccarty deleted the ci-fixes branch May 27, 2022 14:13

This was referenced May 27, 2022

[Do Not Merge]Test CI build #913

Closed

[WIP]Fix Dask-ML CI #924

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing CI #929

Fixing CI #929

mmccarty commented May 20, 2022 •

edited

Loading

mmccarty commented May 24, 2022

mmccarty commented May 25, 2022

wence- left a comment

VibhuJawa May 25, 2022

mmccarty May 25, 2022

wence- May 26, 2022

VibhuJawa May 26, 2022

VibhuJawa May 26, 2022

mmccarty May 27, 2022

VibhuJawa May 25, 2022

VibhuJawa May 25, 2022

VibhuJawa May 25, 2022

wence- May 26, 2022

TomAugspurger May 27, 2022

wence- left a comment

TomAugspurger May 27, 2022

TomAugspurger commented May 27, 2022 •

edited

Loading

mmccarty commented May 27, 2022

	rng2 = (
	random_state.randint(0, np.iinfo("i4").max - 1, chunks=())
	.compute(scheduler="single-threaded")
	.item()

Fixing CI #929

Fixing CI #929

Conversation

mmccarty commented May 20, 2022 • edited Loading

mmccarty commented May 24, 2022

mmccarty commented May 25, 2022

wence- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wence- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomAugspurger commented May 27, 2022 • edited Loading

mmccarty commented May 27, 2022

mmccarty commented May 20, 2022 •

edited

Loading

TomAugspurger commented May 27, 2022 •

edited

Loading