Talip7/scikit-learn-issues-multilabel huggingface dataset:
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
Cross referencing #31368

Labels: Array API
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
can someone cross verify my PR?

Labels: Array API
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
Wow good catch, this seems like indeed something to keep in mind ...

Labels: Array API
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
I think this (and https://github.com/scikit-learn/scikit-learn/issues/31368) may only be a problem when we want to add support for other array libraries - ones that don't follow numpy as much.

Labels: Array API
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
I think we should at least add `# XXX: the following assume that all array API namespaces follow the NumPy ordering semantics.` and maybe link to the spec to make it explicit that this is not guaranteed.

Maybe we could add a helper to post-sort the unique values for array API namespaces that are not known to follow the NumPy ordering semantics.

Labels: Array API
Array API spec for `unique_values` does not specify order of unique elements

Noticed when working on #32923

I don't think we need to do anything about this, (and indeed all our tests pass), I just wanted to document this for posterity, in case we encounter any problems later.

Array API spec for `unique_values` does not specify what the order of unique elements should be.
From https://data-apis.org/array-api/latest/API_specification/generated/array_api.unique_values.html#array_api.unique_values :

> The order of unique elements is not specified and may vary between implementations.

Searching for `xp.unique_values` in our codebase, I can see 2 places where order would matter.

In `_check_pos_label_consistency` we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/validation.py#L2578-L2582

which assumes a specific order of unique values.

Also in `_unique_np` (used in `_encode.py`), we do:

https://github.com/scikit-learn/scikit-learn/blob/86acf4547ed8e183cb75c07bcd68ef186a223f06/sklearn/utils/_encode.py#L71-L73

assuming NaNs get sorted to the end.

COMMENT:
Hello everyone I have never contributed to sickit Learn before , can I please work on this problem ?

Labels: Array API
[for discussion]: adding link to Contributing Guide to top menu

### Describe the issue linked to the documentation

This issue is a **discussion** item.

## Questions:  
1. Can we add a link to "Contributing" to the top level menu? It seems a bit hidden, and the questions on contributing come up quite often.
    - Contributing: https://scikit-learn.org/stable/developers/contributing.html#contributing

I usually find the link for it via the FAQs: Contributing: https://scikit-learn.org/stable/faq.html#contributing

<img width="836" height="366" alt="Image" src="https://github.com/user-attachments/assets/c7582fd5-4937-466a-bf34-3a5db3bdaf11" />

2. Should we add a section indicating that scikit-learn no longer participates in GSOC?



## Note for New Contributors

> [!WARNING]
>  This issue is for discussion amongst the maintainers. Please do not ask if you can work on it or to be assigned to it. 

### Suggest a potential alternative/fix

_No response_

COMMENT:
Why not, I guess there is a number of links already but maybe it's fine to add another one? I personally find it through Development and then Contributing is the first link.

I think it would be worth trying to make the [Development page](https://scikit-learn.org/dev/developers/index.html) more useful. IMO matplotlib is a great [example to follow](https://matplotlib.org/devdocs/devel/index). One thing matplotlib has done is to have a top-level Contribute link on https://matplotlib.org (which actually goes to their development doc but the index is centered around contributing which makes sense). Maybe we could do something similar and rename the Development link to Contributing?

Tangentially related but longer term I also have a plan to reorganize the contributing doc, because there is definitely room for improvement on this side. For now, this is in a [pad](https://hackmd.io/X8T3WmGBRj6g4DTUxhEaoA), which a few other people have contributed to.

Labels: Documentation
[for discussion]: adding link to Contributing Guide to top menu

### Describe the issue linked to the documentation

This issue is a **discussion** item.

## Questions:  
1. Can we add a link to "Contributing" to the top level menu? It seems a bit hidden, and the questions on contributing come up quite often.
    - Contributing: https://scikit-learn.org/stable/developers/contributing.html#contributing

I usually find the link for it via the FAQs: Contributing: https://scikit-learn.org/stable/faq.html#contributing

<img width="836" height="366" alt="Image" src="https://github.com/user-attachments/assets/c7582fd5-4937-466a-bf34-3a5db3bdaf11" />

2. Should we add a section indicating that scikit-learn no longer participates in GSOC?



## Note for New Contributors

> [!WARNING]
>  This issue is for discussion amongst the maintainers. Please do not ask if you can work on it or to be assigned to it. 

### Suggest a potential alternative/fix

_No response_

COMMENT:
> Maybe we could do something similar and rename the Development link to Contributing?

I think I would be +1 for this

For sections in 'Development' that cover specific topics surrounding development (e.g., testing, CI, backwards compatibility) - maybe we could include this in a different section, potentially a new 'Development' section ?

Labels: Documentation
[for discussion]: adding link to Contributing Guide to top menu

### Describe the issue linked to the documentation

This issue is a **discussion** item.

## Questions:  
1. Can we add a link to "Contributing" to the top level menu? It seems a bit hidden, and the questions on contributing come up quite often.
    - Contributing: https://scikit-learn.org/stable/developers/contributing.html#contributing

I usually find the link for it via the FAQs: Contributing: https://scikit-learn.org/stable/faq.html#contributing

<img width="836" height="366" alt="Image" src="https://github.com/user-attachments/assets/c7582fd5-4937-466a-bf34-3a5db3bdaf11" />

2. Should we add a section indicating that scikit-learn no longer participates in GSOC?



## Note for New Contributors

> [!WARNING]
>  This issue is for discussion amongst the maintainers. Please do not ask if you can work on it or to be assigned to it. 

### Suggest a potential alternative/fix

_No response_

COMMENT:
Just to try to be 100% explicit, for simplicity I would the same as matplotlib, keep the same URL <https://scikit-learn.org/dev/developers/index.html> but change the content of the index to be centered around contributing. Right now it's only a TOC that is a bit hard to read but maybe some people find it useful not 100% sure ...

Longer-term I'd like to at least try to move/reorder things a bit but having an index centered on contributing would be already a great win IMO.

This is what matplotlib is doing <https://matplotlib.org/devdocs/devel/index.html> is where the Contribute link points to and is the development guide, as you can tell from `devel` in the URL and also from some sections on the LHS: "Coding guidelines", "Release guide", etc ... The index.html content on the other hand is focused on Contributing which makes sense.

Labels: Documentation
[for discussion]: adding link to Contributing Guide to top menu

### Describe the issue linked to the documentation

This issue is a **discussion** item.

## Questions:  
1. Can we add a link to "Contributing" to the top level menu? It seems a bit hidden, and the questions on contributing come up quite often.
    - Contributing: https://scikit-learn.org/stable/developers/contributing.html#contributing

I usually find the link for it via the FAQs: Contributing: https://scikit-learn.org/stable/faq.html#contributing

<img width="836" height="366" alt="Image" src="https://github.com/user-attachments/assets/c7582fd5-4937-466a-bf34-3a5db3bdaf11" />

2. Should we add a section indicating that scikit-learn no longer participates in GSOC?



## Note for New Contributors

> [!WARNING]
>  This issue is for discussion amongst the maintainers. Please do not ask if you can work on it or to be assigned to it. 

### Suggest a potential alternative/fix

_No response_

COMMENT:
Hi, I’m Sindhu. I work with Python and ML and would like to take this documentation issue.
I plan to update the relevant docs and add clarity/examples where needed.
Please let me know if that approach works. Thanks!

Labels: Documentation
[for discussion]: adding link to Contributing Guide to top menu

### Describe the issue linked to the documentation

This issue is a **discussion** item.

## Questions:  
1. Can we add a link to "Contributing" to the top level menu? It seems a bit hidden, and the questions on contributing come up quite often.
    - Contributing: https://scikit-learn.org/stable/developers/contributing.html#contributing

I usually find the link for it via the FAQs: Contributing: https://scikit-learn.org/stable/faq.html#contributing

<img width="836" height="366" alt="Image" src="https://github.com/user-attachments/assets/c7582fd5-4937-466a-bf34-3a5db3bdaf11" />

2. Should we add a section indicating that scikit-learn no longer participates in GSOC?



## Note for New Contributors

> [!WARNING]
>  This issue is for discussion amongst the maintainers. Please do not ask if you can work on it or to be assigned to it. 

### Suggest a potential alternative/fix

_No response_

COMMENT:
@sindhukodati  Please wait for consensus to be reached first.

Labels: Documentation
Addressed: FastICA logcosh bottleneck

### Describe the workflow you want to enable

For: sklearn/decomposition/_fastica.py

Hello,

Firstly thank you for the amazing library :D

I see in the FastICA there was a bottleneck that was messing a lot with my run time, so i optimized it and compared my implementation with the original and the results are exactly the same. I think you may find it useful. 


### Describe your proposed solution


In my implementation I use logcocsh as a callable


Original Scikit:
def _logcosh(x, fun_args=None):
    alpha = fun_args.get("alpha", 1.0)  # comment it out?

    x *= alpha
    gx = np.tanh(x, x)  # apply the tanh inplace
    g_x = np.empty(x.shape[0], dtype=x.dtype)
    # XXX compute in chunks to avoid extra allocation
    for i, gx_i in enumerate(gx):  # please don't vectorize.
        g_x[i] = (alpha * (1 - gx_i**2)).mean()
    return gx, g_x


My implementation:
def opt_logcosh(x):
    """
    we create our own logcosh, which is modernized and avoids the bottleneck of the original logcosh

    our logcosh vs the original logcosh from sklearn:
        same output, but faster execution time

    """
    tanh = np.tanh(x)
    return tanh, ((1 - tanh**2)).mean(axis=-1)

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

_No response_

### Additional context

_No response_

COMMENT:
Thanks, that looks like a legitimate area of improvement. `_logcosh` implementation is very different from the two other options `_exp` and `_cube` , and doesn't respect what's written in the docstring for `fun` for 1D arrays: "The derivative should be averaged along its last dimension."

But first, can you clean-up the formatting of your issue (code blocks)? Also, can you give the parameters you're using in FastICA? And ideally, a reproducer showing the slowness of the current implementation, and the improvement you get by using your own logcosh.

Labels: New Feature, Needs Triage
Addressed: FastICA logcosh bottleneck

### Describe the workflow you want to enable

For: sklearn/decomposition/_fastica.py

Hello,

Firstly thank you for the amazing library :D

I see in the FastICA there was a bottleneck that was messing a lot with my run time, so i optimized it and compared my implementation with the original and the results are exactly the same. I think you may find it useful. 


### Describe your proposed solution


In my implementation I use logcocsh as a callable


Original Scikit:
def _logcosh(x, fun_args=None):
    alpha = fun_args.get("alpha", 1.0)  # comment it out?

    x *= alpha
    gx = np.tanh(x, x)  # apply the tanh inplace
    g_x = np.empty(x.shape[0], dtype=x.dtype)
    # XXX compute in chunks to avoid extra allocation
    for i, gx_i in enumerate(gx):  # please don't vectorize.
        g_x[i] = (alpha * (1 - gx_i**2)).mean()
    return gx, g_x


My implementation:
def opt_logcosh(x):
    """
    we create our own logcosh, which is modernized and avoids the bottleneck of the original logcosh

    our logcosh vs the original logcosh from sklearn:
        same output, but faster execution time

    """
    tanh = np.tanh(x)
    return tanh, ((1 - tanh**2)).mean(axis=-1)

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

_No response_

### Additional context

_No response_

COMMENT:
Here is the code to test that the speedup is around 11x and the results are the same


```
import numpy as np
from sklearn.decomposition import FastICA
import time

def opt_logcosh(x):
    tanh = np.tanh(x)
    return tanh, (1 - tanh**2).mean(axis=-1)

rng = np.random.RandomState(0)

# Match real use case: 130 channels × 50 features
X = rng.randn(130, 50)





t0 = time.time()
ica = FastICA(n_components=50,
            algorithm='deflation', 
            fun="logcosh",
            tol=1e-6,
            max_iter=10000, 
            random_state=0)
ica.fit(X)
t_ref = time.time() - t0

t0 = time.time()
ica_opt = FastICA(n_components=50,
                algorithm='deflation', 
                fun=opt_logcosh,
                tol=1e-6, 
                max_iter=10000, 
                random_state=0)
ica_opt.fit(X)
t_opt = time.time() - t0

print(f"Reference implementation time: {t_ref:.4f} seconds")
print(f"Optimized implementation time: {t_opt:.4f} seconds")
print(f"Speedup: {t_ref / t_opt:.2f}x")

print("Checking that the results are similar...")
np.testing.assert_allclose(ica.components_, ica_opt.components_, atol=1e-100)
print("Results are similar within the tolerance.")
```

In my machine:
Reference implementation time: 0.8266 seconds
Optimized implementation time: 0.0786 seconds
Speedup: 10.51x
Checking that the results are similar...
Results are similar within the tolerance.

Labels: New Feature, Needs Triage
Addressed: FastICA logcosh bottleneck

### Describe the workflow you want to enable

For: sklearn/decomposition/_fastica.py

Hello,

Firstly thank you for the amazing library :D

I see in the FastICA there was a bottleneck that was messing a lot with my run time, so i optimized it and compared my implementation with the original and the results are exactly the same. I think you may find it useful. 


### Describe your proposed solution


In my implementation I use logcocsh as a callable


Original Scikit:
def _logcosh(x, fun_args=None):
    alpha = fun_args.get("alpha", 1.0)  # comment it out?

    x *= alpha
    gx = np.tanh(x, x)  # apply the tanh inplace
    g_x = np.empty(x.shape[0], dtype=x.dtype)
    # XXX compute in chunks to avoid extra allocation
    for i, gx_i in enumerate(gx):  # please don't vectorize.
        g_x[i] = (alpha * (1 - gx_i**2)).mean()
    return gx, g_x


My implementation:
def opt_logcosh(x):
    """
    we create our own logcosh, which is modernized and avoids the bottleneck of the original logcosh

    our logcosh vs the original logcosh from sklearn:
        same output, but faster execution time

    """
    tanh = np.tanh(x)
    return tanh, ((1 - tanh**2)).mean(axis=-1)

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

_No response_

### Additional context

_No response_

COMMENT:
@fkay-codec You could further do operations in-place. 

```
# id(x) 
np.tanh(x, out=x)
x **= 2
x *= -1
x += 1
# id(x), same id
x.mean(axis=-1)
```

There is some discussion on original issue for the original implementation:
https://github.com/scikit-learn/scikit-learn/pull/2290

Labels: New Feature, Needs Triage
Addressed: FastICA logcosh bottleneck

### Describe the workflow you want to enable

For: sklearn/decomposition/_fastica.py

Hello,

Firstly thank you for the amazing library :D

I see in the FastICA there was a bottleneck that was messing a lot with my run time, so i optimized it and compared my implementation with the original and the results are exactly the same. I think you may find it useful. 


### Describe your proposed solution


In my implementation I use logcocsh as a callable


Original Scikit:
def _logcosh(x, fun_args=None):
    alpha = fun_args.get("alpha", 1.0)  # comment it out?

    x *= alpha
    gx = np.tanh(x, x)  # apply the tanh inplace
    g_x = np.empty(x.shape[0], dtype=x.dtype)
    # XXX compute in chunks to avoid extra allocation
    for i, gx_i in enumerate(gx):  # please don't vectorize.
        g_x[i] = (alpha * (1 - gx_i**2)).mean()
    return gx, g_x


My implementation:
def opt_logcosh(x):
    """
    we create our own logcosh, which is modernized and avoids the bottleneck of the original logcosh

    our logcosh vs the original logcosh from sklearn:
        same output, but faster execution time

    """
    tanh = np.tanh(x)
    return tanh, ((1 - tanh**2)).mean(axis=-1)

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

_No response_

### Additional context

_No response_

COMMENT:
Thanks, that's a good point! My goal here was mainly to demonstrate the performance impact and provide a minimal reproducer.

Labels: New Feature, Needs Triage