Talip7/scikit-learn-issues-multilabel huggingface dataset:
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 don't think we should raise a warning, unless we can identify clear cases where missing values during test time is clearly unexpected.

There's no right or wrong strategy here I believe but we should note that the strategy suggested above means that `predict(X)` called twice on the same `X` may lead to different predictions. Do we have other predictors in scikit-learn that are inherently random (excluding tie-breaking etc.)?

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:
Relevant paper showing empirical evidence that that sending samples to the majority child node is not as good as "random" when the sample contains an unseen category during training. In the context of missing values, a missing value indicator is a binary category.

https://jmlr.csail.mit.edu/papers/volume19/16-474/16-474.pdf#page14

## summary

The paper looks at categorical splits and not seeing a certain category during training and what to do during predict time. This is termed the "absent level" problem. This is analogous to not seeing missing values during training but during testing. Which child node do we send that sample down?

The author runs experiments showing that randomly sending the sample with an absent level is the best unbiased heuristic.

I think it's reasonable to impose that logic during test time.

## proposal

Extend the missing_go_to_left parameter with a third possibility where it indicates that the sample should be randomly sent down the tree. This only occurs if no missing values during training but there are missing values during prediction.

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 spoke with @betatim today about this issue, and to summarize, I think a good strategy is the following (hopefully he agrees :p):

If no missing-values are encountered during training, then flip a coin and set the missing value traversal to be random. The LOC that would require changing would be [here](https://github.com/scikit-learn/scikit-learn/blob/6bf2061f76ba0977c1f7ffb9ddc48db794e5c7ec/sklearn/tree/_splitter.pyx#L473), which would involve a random number generation. The sampling strategy would change to:

```
if n_missing == 0:
        current_split.missing_go_to_left = rand_uniform(0, 1, random_state) >= 0.5
```

There should be no "performance hit really" as we're just setting the trees to be "unbiased" wrt missing values at inference time. This is the most fair and simple approach I could think of. If there is a performance hit, we can also have this be a user-defined argument: `missing_random_at_inference=False`?

cc: @thomasjpfan do you have a strong opinion here?

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:
API wise, I'll be okay with flipping a coin during training time and adding a flag to trigger it so we stay backward compatible.

> If no missing-values are encountered during training, then flip a coin and set the missing value traversal to be random. 

Is there literature that backs this technique?

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 no missing-values are encountered during training, then flip a coin and set the missing value traversal to be random.
> 
> Is there literature that backs this technique?

I would say not directly this specific method in trees. This motivates an interesting research project of empirically highlighting the bias that can occur when missing-values occur completely at random during inference time, and how by sending the missing-values to the node with more samples, it could be wrong.

Re assumptions related to missing-values, the literature in causal inference related to missingness is highly relevant. With that being said, I do think that the idea that missing-ness influences predictions at inference time (if not encountered during training) is a bug, and we should flip a coin rather than send it to the node with the most samples.

Labels: Needs Investigation
⚠️ CI failed on Linux_nogil.pylatest_pip_nogil ⚠️

**CI is still failing on [Linux_nogil.pylatest_pip_nogil](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65622&view=logs&j=67fbb25f-e417-50be-be55-3b1e9637fce5)** (Apr 08, 2024)
Unable to find junit file. Please see link for details.

COMMENT:
Likely due to a Cython update: https://github.com/scikit-learn/scikit-learn/pull/28743.

Waiting for https://github.com/colesbury/nogil-wheels/pull/8 to be merged.

Labels: Bug
⚠️ CI failed on Linux_nogil.pylatest_pip_nogil ⚠️

**CI is still failing on [Linux_nogil.pylatest_pip_nogil](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65622&view=logs&j=67fbb25f-e417-50be-be55-3b1e9637fce5)** (Apr 08, 2024)
Unable to find junit file. Please see link for details.

COMMENT:
> Waiting for https://github.com/colesbury/nogil-wheels/pull/8 to be merged.

It has been merged, and Cython 3.0.10 is available for nogil. I will double-check tomorrow that this fixes the issue.

Labels: Bug
⚠️ CI failed on Linux_nogil.pylatest_pip_nogil ⚠️

**CI is still failing on [Linux_nogil.pylatest_pip_nogil](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65622&view=logs&j=67fbb25f-e417-50be-be55-3b1e9637fce5)** (Apr 08, 2024)
Unable to find junit file. Please see link for details.

COMMENT:
## CI is no longer failing! ✅

[Successful run](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65671&view=logs&j=67fbb25f-e417-50be-be55-3b1e9637fce5) on Apr 09, 2024

Labels: Bug
Reduce ninja's verbosity from subprocesses

When importing scikit-learn, even if there's nothing to recompile, ninja will output
```
+ /home/jeremie/miniforge/envs/dev/bin/ninja
ninja: no work to do.
```
This is acceptable, but when using an estimator that uses multiprocessing, it will be printed for each sub-process which can be a bit annoying (and mixed with the prints of said estimator)

COMMENT:
Yep, this is https://github.com/mesonbuild/meson-python/pull/594 :wink:. Hopefully this will be merged soonish and a new meson-python will be made (it also has https://github.com/mesonbuild/meson-python/pull/569 that we care about, this is the reason we are using pytest<8 in the CI). My understanding is that Ralf Gommers is the only possible reviewer and that with Numpy 2.0 migration he has other priorities, which is completetely understandable.

Note you can build without verbose if you are too annoyed (remove the flag in the `pip` install command), there is also an environment variable you can use `MESONPY_EDITABLE_VERBOSE` https://meson-python.readthedocs.io/en/latest/how-to-guides/editable-installs.html#verbose-mode. I am not sure which one takes precedence to be honest.

Closing this one, because the problem will likely be fixed in the not so distant future :crossed_fingers:.

Labels: Needs Triage
Reduce ninja's verbosity from subprocesses

When importing scikit-learn, even if there's nothing to recompile, ninja will output
```
+ /home/jeremie/miniforge/envs/dev/bin/ninja
ninja: no work to do.
```
This is acceptable, but when using an estimator that uses multiprocessing, it will be printed for each sub-process which can be a bit annoying (and mixed with the prints of said estimator)

COMMENT:
meson-python 0.16 has been released on April 16 and fixes this :tada:

Labels: Needs Triage
Installing from source issue

When following the guidelines for **installing scikit-learn from source** (https://scikit-learn.org/dev/developers/advanced_installation.html#building-from-source), I encountered the a problem at step 5:

```bash
pip install -v --no-use-pep517 --no-build-isolation -e .
```

which leads to the following error:

```bash
Using pip 24.0 from /Users/josephbarbier/opt/anaconda3/envs/sklearn-dev/lib/python3.12/site-packages/pip (python 3.12)
Obtaining file:///Users/josephbarbier/Desktop/scikit-learn
ERROR: Disabling PEP 517 processing is invalid: project specifies a build backend of mesonpy in pyproject.toml
```

I also tried:

```bash
pip install -v -e .
```

and then I get (at the end):

```bash
Successfully built scikit-learn
Installing collected packages: threadpoolctl, joblib, scikit-learn
Successfully installed joblib-1.3.2 scikit-learn-1.5.dev0 threadpoolctl-3.4.0
```

But then I run:

```bash
python -c "import sklearn; sklearn.show_versions()"
```

I get:

```bash
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 982, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 925, in _find_spec
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/site-packages/_scikit_learn_editable_loader.py", line 271, in find_spec
    tree = self.rebuild()
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/site-packages/_scikit_learn_editable_loader.py", line 312, in rebuild
    subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True)
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 505, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 1837, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/private/var/folders/m2/jfzxyg4s0xz3jyw5f94r7tcm0000gn/T/pip-build-env-kba2z47e/normal/bin/ninja'
```

### Additional context

- `conda list` includes both `compilers` and `llvm-openmp`
- both `conda` and `pip` guides lead to the same error
- macOS Sonoma 14.4
- `Python 3.9.19` and `pip 24.0`

### Suggest a potential alternative/fix

_No response_

COMMENT:
Looking at the documentation, I think this is a bit confusing because we end-up first in the following section: https://scikit-learn.org/dev/developers/advanced_installation.html#editable-mode where we will get the error.

I'm wondering, if we should only document the new way using meson python and add a dropdown menu that is folder by default that would be the "legacy" mode using setuptools.

Labels: Documentation
Installing from source issue

When following the guidelines for **installing scikit-learn from source** (https://scikit-learn.org/dev/developers/advanced_installation.html#building-from-source), I encountered the a problem at step 5:

```bash
pip install -v --no-use-pep517 --no-build-isolation -e .
```

which leads to the following error:

```bash
Using pip 24.0 from /Users/josephbarbier/opt/anaconda3/envs/sklearn-dev/lib/python3.12/site-packages/pip (python 3.12)
Obtaining file:///Users/josephbarbier/Desktop/scikit-learn
ERROR: Disabling PEP 517 processing is invalid: project specifies a build backend of mesonpy in pyproject.toml
```

I also tried:

```bash
pip install -v -e .
```

and then I get (at the end):

```bash
Successfully built scikit-learn
Installing collected packages: threadpoolctl, joblib, scikit-learn
Successfully installed joblib-1.3.2 scikit-learn-1.5.dev0 threadpoolctl-3.4.0
```

But then I run:

```bash
python -c "import sklearn; sklearn.show_versions()"
```

I get:

```bash
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 982, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 925, in _find_spec
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/site-packages/_scikit_learn_editable_loader.py", line 271, in find_spec
    tree = self.rebuild()
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/site-packages/_scikit_learn_editable_loader.py", line 312, in rebuild
    subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True)
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 505, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/Users/josephbarbier/opt/anaconda3/envs/sklearn-env/lib/python3.9/subprocess.py", line 1837, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/private/var/folders/m2/jfzxyg4s0xz3jyw5f94r7tcm0000gn/T/pip-build-env-kba2z47e/normal/bin/ninja'
```

### Additional context

- `conda list` includes both `compilers` and `llvm-openmp`
- both `conda` and `pip` guides lead to the same error
- macOS Sonoma 14.4
- `Python 3.9.19` and `pip 24.0`

### Suggest a potential alternative/fix

_No response_

COMMENT:
> I'm wondering, if we should only document the new way using meson python and add a dropdown menu that is folder by default that would be the "legacy" mode using setuptools.

+1

Labels: Documentation