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

Allowing for models without built-in or custom scoring to be used #923

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nlahaye
Copy link

@nlahaye nlahaye commented Apr 27, 2022

Hello,
I hope you are all doing well!

Currently, models that do not have built-in scoring cannot be used with dask_ml wrappers, unless custom scoring is specified. For example, using this code, taken mostly from the sklearn example here

import numpy as np
import dask.array as da
from sklearn.cluster import Birch
from dask_ml.wrappers import Incremental
from sklearn.datasets import make_blobs


xx = da.linspace(-22, 22, 10)
yy = da.linspace(-22, 22, 10)
xx, yy = da.meshgrid(xx, yy)
n_centers = da.hstack((da.ravel(xx)[:, np.newaxis], da.ravel(yy)[:, np.newaxis]))

X, y = make_blobs(n_samples=25000, centers=n_centers, random_state=0)
birch =  Birch(threshold=1.7, n_clusters=None)
inc = Incremental(birch)
inc.fit(X)

This code currently, raises a TypeError because: If no scoring is specified, the estimator passed should have a 'score' method. The estimator Birch(n_clusters=None, threshold=1.7) does not.

I fixed it by allowing for None when calling check_scoring within the fit method. If there is a reason to not allow a loosening of this check, I am open to other solutions. Other potential solutions I saw at a glance include:

  1. adding a parameter to fit that propagates to the allow_none value of check_scoring - defaulted to False
  2. removing the line, as I do not see scoring being used directly in _fit_for_estimator, and other areas of the code explicitly check for/handle scoring = None.

All tests that utilize classes within dask_ml.wrapper pass.

Thanks!
Nick

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

Successfully merging this pull request may close these issues.

1 participant