My criteria for changing the default: where would IncrementalSearchCV provide the most value?

I think it provides the most value in "memory but not compute constrained" problems. This means that decay_rate should default to 0 because this isn't compute constrained.

The reason I think it fits best here is because IncrementalSearchCV works better for large datasets. It can go over the entire dataset because it provides access to partial_fit. dask-searchcv can't do this because it only has access to fit, so it fits to each chunk of the data independently (_search.py#L416-L422). I should add to dask-searchcv's docs to reflect this.

Without changing the default, there isn't a class that fits into "memory but not compute constrained". I don't think DaskGridSearchCV(Incremental(model), params) would work, especially if more than one pass through data wanted.

This idea matches the documentation:

dask_ml.wrappers.Incremetnal currently does not work well with hyper-parameter optimization like sklearn.model_selection.GridSearchCV. If you need to do hyper-parameter optimization on larger-than-memory datasets, we recommend dask_ml.model_selection.IncrementalSearch

—http://ml.dask.org/incremental.html

TODO:

• add to dask-searchcv documentation to clarify how Dask arrays are handled
• change default decay_rate of IncrementalSearchCV
• take dask-searchcv off "memory but not compute constrained" (but add a note below).

it fits to each chunk of the data independently (_search.py#L416-L422). I should add to dask-searchcv's docs to reflect this.

I don't think that's correct. The algorithms in _search.py mirror scikit-learn. I believe that the Xs and ys you linked to are replicas of the training dataset (e.g.

Thanks for the review @TomAugspurger! I found it useful, especially the wording choices.

The actual fit calls will take place on CV splits of the training dataset.

Right. I've written the docs to mention that each CV split is gathered to one machine for most estimators. Can you review this?

I know dask_ml.Incremental aliases fit to call partial_fit on each chunk of the Dask array. Would that work with dask-searchcv's searches? I believe dask-searchcv computes each CV split regardless of what fit can take, but I'm having a hard time seeing that.

This PR now depends on #221 (but implicitly, which is why the Travis CI tests are failing).

I'm drafting a paper for the SciPy 2019 proceedings. In this, I explicitly mention that decay_rate=0 by default. I use this to show that IncrementalSearchCV is best suited for memory-constrained problems.

To the extent possible, I'd recommend keeping documentation changes, additional tests, and behavior changes separate. That makes things much easier for me to review.

I've tried to do that with two PRs:

• This PR is focused around documentation improvements to docs/source/hyperparameter.rst. In addition to making these improvements, this PR also...
  • Deprecates decay_rate. This makes IncrementalSearchCV better fit in "memory but not compute constrained".
  • Implements a new class InverseDecaySearchCV. This class is a wrapper for the previous implementation of IncrementalSearchCV._adapt.
  • Makes other small documentation improvements (adding to documentation of dask-searchcv, linking in Hyperband docs, etc)
• MAINT: Clean InverseDecaySearchCV #651: cleaning InverseDecaySearchCV. This PR implements and tests improvements to InverseDecaySearchCV._adapt.

Will that be sufficient, or should I remove the IncrementalSearchCV deprecation as well? That'll mean this PR is only modifies *.rst files/docstrings and #651 only modifies *.py files.

Here are some arguments for deprecating decay_rate:

1. (documentation) IncrementalSearchCV is the only class to scale "memory- but not compute-constrained" searches. I think these searches is important and common enough to include an implementation that works by default..
   
   I would imagine this is the most prominent usage of Dask-ML's hyperparameter optimization: "memory- but not compute constrained" includes training linear models on large datasets.
2. (API) decay_rate is a algorithm specific parameter. Other adaptive algorithms won't need decay_rate. Case in point: SuccessiveHalvingSearchCV doesn't take decay_rate. Other adaptive algorithms like (the popular) median elimination won't either. This leads to a violation of Liskov substitution principle.
3. (weaker API) Without deprecating decay_rate, IncrementalSearchCV will have two useful implementations:
   • An implementation best suited for large data with decay_rate=0
   • An implementation of an adaptive algorithm with decay_rate=1 (at least after MAINT: Clean InverseDecaySearchCV #651).

I don't know how decay_rate performs if decay_rate not in [0, 1]. I don't even really know how decay_rate=1 performs, but at least know the motivation and have performance results for a related implementation, SuccessiveHalvingSearchCV.

(1) is the motivation for included the deprecation in this PR.

FYI, going to ignore the sklearn dev failures for now. We'll need to add n_features_in_ to each estimator, which will take a bit of work.

It looks like all the CI failures are unrelated. There's a couple other warnings besides the one with n_features_in_:

n_features_in_:

# data = dask.array<array, shape=(10, 4), dtype=float64, chunksize=(5, 4), chunktype=numpy.ndarray>

@pytest.mark.parametrize("data", [X, dX, df, ddf])
def test_fit(data):
    a = sklearn.impute.SimpleImputer()
    b = dask_ml.impute.SimpleImputer()
    
    a.fit(X)
    b.fit(data)
    
    assert_estimator_equal(a, b)

PartialPassiveAggressiveClassifier.average_intercept_:

# file: TestPassiveAggressiveClassifier.test_basic

def test_basic(self, single_chunk_classification):
    X, y = single_chunk_classification
    a = lm.PartialPassiveAggressiveClassifier(classes=[0, 1], random_state=0, max_iter=100, tol=1e-3)
    b = lm_.PassiveAggressiveClassifier(random_state=0, max_iter=100, tol=1e-3)
    a.fit(X, y)
    b.partial_fit(*dask.compute(X, y), classes=[0, 1])
    assert_estimator_equal(a, b, exclude=["loss_function_"])

Passing keyword arguments:

    def test_check_cv():
        # No y, classifier=False
       cv = check_cv(3, classifier=False)

# ______________________ TestPolynomialFeatures.test_basic _______________________

# self = <tests.preprocessing.test_data.TestPolynomialFeatures object at 0x7ff8848d8a90>

def test_basic(self):
    a = dpp.PolynomialFeatures()

There are lots of warnings about keyword arguments and n_features_in_.

I fixed the PartialPassiveAggressiveClassifier.average_intercept_ in another branch.

I think this is ready to go. All good on your end?

Going to merge so we can include this in the 1.4.0 release. I can make another release anytime if needed.

I think this is ready to go. All good on your end?

Yup! I'm glad to see this merged.