Support weights="distance" for KNeighbors* in cuml.accel

[jcrist]

Previously we would fail if the user specified weights="distance" to KNeighborsClassifier/KNeighborsRegressor. This fixes that and adds a test.

Part of fixing this required changing the logic in dispatch_func to not special-case inference methods. Previously we would always run inference on the GPU, even if _gpuaccel was False (meaning the hyperparameters specified weren't supported by cuml). I don't believe this to be the desired logic - if cuml doesn't support the specified hyperparameters, we cannot be sure that we do support them for predict. Further, the state is stored on the cpu estimator already, running inference on CPU makes more sense anyway IMO. It also makes understanding where something runs clearer:

• If the hyperparameters aren't supported by cuml, then everything runs on CPU
• If the arguments provided to a method aren't supported by cuml, then that method will dispatch to CPU
• Otherwise we run on GPU

Fixes #6545.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[csadorf]

I ran the scikit-learn test suite against this with the regression testing that I'm currently implementing in #6553. This is what I found:

Test Summary:
  Total Tests:             36096
  Passed:                  30994
  Failed:                      1
  XFailed:                  1279
  XPassed (strict):            9
  XPassed (non-strict):        0
  Errors:                      0
  Skipped:                  3813
  Pass Rate:              85.87%
  Total Time:            267.86s

Failed Tests:
  test_regression_criterion[absolute_error-RandomForestRegressor]

Potential Improvements (Strict XPASS):
  test_knn_imputer_weight_distance[nan]
  test_knn_imputer_weight_distance[-1]
  test_neighbors_regressors_zero_distance
  test_neighbors_metrics[float64-42-mahalanobis]
  test_valid_brute_metric_for_auto_algorithm[float64-csr_matrix-mahalanobis]
  test_kneighbors_brute_backend[float64-42-mahalanobis]
  test_valid_brute_metric_for_auto_algorithm[float64-csr_array-mahalanobis]
  test_ovo_consistent_binary_classification
  test_unsupervised_model_fit[2]

That's overall very positive!

However, it looks like the test_regression_criterion[absolute_error-RandomForestRegressor] regression might be real, because it goes away when I revert the change to the base.pyx module.

Traceback

ensemble/tests/test_forest.py::test_regression_criterion[absolute_error-RandomForestRegressor] FAILED                                                                                   [100%]

========================================================================================== FAILURES ===========================================================================================
_______________________________________________________________ test_regression_criterion[absolute_error-RandomForestRegressor] _______________________________________________________________

name = 'RandomForestRegressor', criterion = 'absolute_error'

    @pytest.mark.parametrize("name", FOREST_REGRESSORS)
    @pytest.mark.parametrize(
        "criterion", ("squared_error", "absolute_error", "friedman_mse")
    )
    def test_regression_criterion(name, criterion):
        # Check consistency on regression dataset.
        ForestRegressor = FOREST_REGRESSORS[name]
    
        reg = ForestRegressor(n_estimators=5, criterion=criterion, random_state=1)
        reg.fit(X_reg, y_reg)
>       score = reg.score(X_reg, y_reg)

../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/sklearn/ensemble/tests/test_forest.py:173: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/cuml/internals/api_decorators.py:219: in wrapper
    return func(*args, **kwargs)
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/nvtx/nvtx.py:122: in inner
    result = func(*args, **kwargs)
randomforestregressor.pyx:691: in cuml.ensemble.randomforestregressor.RandomForestRegressor.score
    ???
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/cuml/internals/api_decorators.py:217: in wrapper
    ret = func(*args, **kwargs)
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/nvtx/nvtx.py:122: in inner
    result = func(*args, **kwargs)
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/cuml/internals/api_decorators.py:369: in dispatch
    return self.dispatch_func(func_name, gpu_func, *args, **kwargs)
../../../miniforge3/envs/cuml-work0/lib/python3.12/site-packages/cuml/internals/api_decorators.py:219: in wrapper
    return func(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   ???
E   TypeError: ForestRegressor.predict() got an unexpected keyword argument 'algo'

base.pyx:757: TypeError

[csadorf]

Great! We will need to address one regression.

[csadorf]

This particular change appears to introduce a regression, see #6554 (comment) .

@dantegd can you comment on the initial motivation for this?

[dantegd]

Technically speaking we can run inference in many cases even if training was done on GPU, but I agree with the change done in this PR after analyzing the behavior that @jcrist mentions. The CPU to GPU transfer eats a lot of the time that the inference acceleration gains.

[jcrist]

Regression should be fixed.

[dantegd]

Technically speaking we can run inference in many cases even if training was done on GPU, but I agree with the change done in this PR after analyzing the behavior that @jcrist mentions. The CPU to GPU transfer eats a lot of the time that the inference acceleration gains.

The regression has been resolved.

[jcrist]

/merge