Talip7/scikit-learn-issues-multilabel huggingface dataset:
Unexpected behavior of sklearn.feature_selection.mutual_info_regression if copy=False

### Describe the bug

The parameter `copy` of the function `mutual_info_regression` is described as follows https://github.com/scikit-learn/scikit-learn/blob/d1d1596fac19d688a637690134d71fc460f5f0dd/sklearn/feature_selection/_mutual_info.py#L381-L383

I read it as both `X` and `y` should be modified if `copy=False` and `X` has continuous features. However, `y`  is always copied. I think the lines 
https://github.com/scikit-learn/scikit-learn/blob/d1d1596fac19d688a637690134d71fc460f5f0dd/sklearn/feature_selection/_mutual_info.py#L309-L310 should be 
```python
    if not discrete_target:
        y = y.astype(np.float64, copy=copy)
        y = scale(y, with_mean=False, copy=False)
```
Similarly to the the treatment of `X` https://github.com/scikit-learn/scikit-learn/blob/d1d1596fac19d688a637690134d71fc460f5f0dd/sklearn/feature_selection/_mutual_info.py#L295-L299

### Steps/Code to Reproduce

```python
import numpy as np
from sklearn.feature_selection import mutual_info_regression
n_samples_, n_feats = 30, 2
X = np.random.randn(n_samples_, n_feats)
y = np.random.randn(n_samples_, )
y_copy = y.copy()
X_copy = X.copy()
mutual_info_regression(X, y, copy=False)
print(np.allclose(y, y_copy), np.allclose(X, X_copy))
```

### Expected Results

The result should be
```
False, False
```
since both `X` and `y` should be modified in place by the function `mutual_info_regression`. 



### Actual Results

```
True, False
```

### Versions

```shell
System:
    python: 3.11.7 (main, Dec  5 2023, 19:13:35) [GCC 10.2.1 20210110]
executable: /usr/local/bin/python
   machine: Linux-5.15.146.1-microsoft-standard-WSL2-x86_64-with-glibc2.31

Python dependencies:
      sklearn: 1.3.0
          pip: 23.3.1
   setuptools: 69.0.2
        numpy: 1.23.2
        scipy: 1.12.0
       Cython: 3.0.9
       pandas: 2.1.4
   matplotlib: 3.8.2
       joblib: 1.3.2
threadpoolctl: 3.2.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 12
         prefix: libopenblas
       filepath: /home/user/.local/lib/python3.11/site-packages/numpy.libs/libopenblas64_p-r0-742d56dc.3.20.so
        version: 0.3.20
threading_layer: pthreads
   architecture: Prescott

       user_api: blas
   internal_api: openblas
    num_threads: 12
         prefix: libopenblas
       filepath: /home/user/.local/lib/python3.11/site-packages/scipy.libs/libopenblasp-r0-23e5df77.3.21.dev.so
        version: 0.3.21.dev
threading_layer: pthreads
   architecture: Prescott

       user_api: openmp
   internal_api: openmp
    num_threads: 12
         prefix: libgomp
       filepath: /home/user/.local/lib/python3.11/site-packages/scikit_learn.libs/libgomp-a34b3233.so.1.0.0
        version: None
```

COMMENT:
Many thanks!

Labels: Documentation, help wanted
Implementing variations of the BIRCH clustering algorithm

### Describe the workflow you want to enable

Currently this only the basic implementation of the BIRCH clustering algorithm. 
https://scikit-learn.org/stable/modules/generated/sklearn.cluster.Birch.html

Just as there is `DBSCAN` and `HDBSCAN`, it would be helpful if there was something like `ABirch` and `MBDBirch` classes as well. 

### Describe your proposed solution

2 additional classes for `ABirch` and `MBDBirch` implementations

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

> [Clustering algorithms](https://www.sciencedirect.com/topics/computer-science/clustering-algorithm) are recently regaining attention with the availability of large datasets and the rise of parallelized [computing architectures](https://www.sciencedirect.com/topics/computer-science/computer-architecture). However, most clustering algorithms suffer from two drawbacks: they do not scale well with increasing dataset sizes and often require proper [parametrization](https://www.sciencedirect.com/topics/computer-science/parametrization) which is usually difficult to provide. A very important example is the cluster count, a parameter that in many situations is next to impossible to assess. In this paper we present A-BIRCH, an approach for automatic threshold estimation for the BIRCH clustering algorithm. This approach computes the optimal threshold parameter of BIRCH from the data, such that BIRCH does proper clustering even without the global clustering phase that is usually the final step of BIRCH. This is possible if the data satisfies certain constraints. If those constraints are not satisfied, A-BIRCH will issue a pertinent warning before presenting the results. This approach renders the final global clustering step of BIRCH unnecessary in many situations, which results in two advantages. First, we do not need to know the expected number of clusters beforehand. Second, without the computationally expensive [final clustering](https://www.sciencedirect.com/topics/computer-science/final-clustering), the fast BIRCH algorithm will become even faster. For very large data sets, we introduce another variation of BIRCH, which we call MBD-BIRCH, which is of particular advantage in conjunction with A-BIRCH but is independent from it and also of general benefit.

https://www.sciencedirect.com/science/article/pii/S2214579617300151

COMMENT:
Thanks for opening an issue!

I am not very familiar with this literature, but I quickly had a look and the paper you mention does not seem to meet our inclusion criteria: https://scikit-learn.org/stable/faq.html#new-algorithms-inclusion-criteria. In particular it has only 100 citations on Google Scholar.

Labels: New Feature
Implementing variations of the BIRCH clustering algorithm

### Describe the workflow you want to enable

Currently this only the basic implementation of the BIRCH clustering algorithm. 
https://scikit-learn.org/stable/modules/generated/sklearn.cluster.Birch.html

Just as there is `DBSCAN` and `HDBSCAN`, it would be helpful if there was something like `ABirch` and `MBDBirch` classes as well. 

### Describe your proposed solution

2 additional classes for `ABirch` and `MBDBirch` implementations

### Describe alternatives you've considered, if relevant

_No response_

### Additional context

> [Clustering algorithms](https://www.sciencedirect.com/topics/computer-science/clustering-algorithm) are recently regaining attention with the availability of large datasets and the rise of parallelized [computing architectures](https://www.sciencedirect.com/topics/computer-science/computer-architecture). However, most clustering algorithms suffer from two drawbacks: they do not scale well with increasing dataset sizes and often require proper [parametrization](https://www.sciencedirect.com/topics/computer-science/parametrization) which is usually difficult to provide. A very important example is the cluster count, a parameter that in many situations is next to impossible to assess. In this paper we present A-BIRCH, an approach for automatic threshold estimation for the BIRCH clustering algorithm. This approach computes the optimal threshold parameter of BIRCH from the data, such that BIRCH does proper clustering even without the global clustering phase that is usually the final step of BIRCH. This is possible if the data satisfies certain constraints. If those constraints are not satisfied, A-BIRCH will issue a pertinent warning before presenting the results. This approach renders the final global clustering step of BIRCH unnecessary in many situations, which results in two advantages. First, we do not need to know the expected number of clusters beforehand. Second, without the computationally expensive [final clustering](https://www.sciencedirect.com/topics/computer-science/final-clustering), the fast BIRCH algorithm will become even faster. For very large data sets, we introduce another variation of BIRCH, which we call MBD-BIRCH, which is of particular advantage in conjunction with A-BIRCH but is independent from it and also of general benefit.

https://www.sciencedirect.com/science/article/pii/S2214579617300151

COMMENT:
Closing in accordance with the above comment.

Labels: New Feature
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
For random forest, I used the same behavior in HistGradientBoosting, which adopted the behavior from LightGBM. @NicolasHug Do you have more context on the missing value behavior for HistGradientBoosting?

Labels: Needs Investigation
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
If it helps the discussion, I found this [old issue](https://github.com/microsoft/LightGBM/issues/2921) in LightGBM, which seems to reflect their docs (I'm unsure cuz I can't find a specific line mentioning how they exactly treat this edge case).

To summarize, it seems the old issue stated they replaced `missing-values` during test (that wasn't seen during training) to '0'. This seems like a reasonable imputation if the feature was 0-centered, but seems a bit odd overall. Having a stochastic treatment as I specified in the proposed soln would:

1. still work similarly to replacing with 0's when the data is 0-centered
2. (more importantly) would work better than replacing with 0's when the data is not 0-centered because of the issues outlined in the issue description.

Lmk if there's some analysis I'm missing wrt LightGBM on perhaps why this behavior is the way it is.

Labels: Needs Investigation
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
Is there any overhead by having the stochastic process? For instance, in case of tie breaking in nearest-neighbors, we did not use a stochastic process because it would be detrimental in terms of computing performance with a huge regression.

Labels: Needs Investigation
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
If it is easy to implement the "make a weighted random choice" option (what you propose) then we could run a benchmark to see what the performance hit is for different cases (many missing values in many features, many missing values in a few features, few missing values in few features, etc). That would give us a way to decide if performance is/isn't something to worry about when making a decision.

Labels: Needs Investigation
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
In terms of an application use case, I'm also wondering if we should not error/warn if a user starts to provide missing values at test time while we did not see any during training. @adam2392 do you have a legitimate use case where this is kind of normal.

Labels: Needs Investigation
BUG(?) Missing-values in RandomForest only during inference time shouldn't send missing-values to the child with most samples

Currently, when missing-values occur only in the testing dataset for constructing a RandomForest, there is a policy that the missing values are sent to the child with the most samples. This amounts to in some sense imputing the missing-value data using the data in the child with the most samples. An issue here is that this may bias the tree prediction towards say a class in the training dataset with more samples. 

For example, say there are 1000 training samples of class 1 and 10 training samples of class 0, and then during test time there are some NaNs. The predictions would then bias towards class 1, whereas it should really be uninformative because the NaNs during test time are treated as missing completely at random.

## Proposed Solution

However, an alternative and more sensible strategy is that when NaNs are not enountered during training, but show up in testing data, they should just be sent stochastically down the tree using weights:

- `p_left_child = n_left_samples / (n_left_samples + n_right_samples)`
- `p_right_child = n_right_samples / (n_left_samples + n_right_samples)`

This ensures that there is no bias towards the class "with more samples". This can be implemented by allowing the value of `missing_go_to_left` (https://github.com/scikit-learn/scikit-learn/blob/6bf0ba5257d55ea0f3c89d7c4762096d84c052cc/sklearn/tree/_splitter.pxd#L28) to be `2`. If the value is `2`, it implies that missing-values were not observed during training time, and thus should be stochastically set.

Overall, it's a very simple change, and I can also implement relevant unit-tests.

cc: @thomasjpfan who implemented the original missing-value support in RandomForest.

## Related
xref: This policy will also impact #27966 and #28268 

This is also an issue in other estimators that handle NaNs: https://scikit-learn.org/stable/modules/ensemble.html#missing-values-support

COMMENT:
I think a warning would be nice but an error message might be overkill because when a NaN pops up in the testing dataset (but not training), ideally our use case is we would like to ignore its effect. If we errored out, we would need to fill in the NaN value or drop the sample altogether. 

OTOH, the issue rn is that by sending it down the child with more samples, it just biases the NaN towards a class (in classification). I also see a case where maybe one would like the current behavior. It all boils down to the assumption on why the NaNs show up. Ideally, both settings should be supported if it's not too complex. 

I can do a quick implementation and try it out on a benchmark tho before continuing the discussion?

Labels: Needs Investigation