AnacondaRecipes / scikit-learn-feedstock

A conda-smithy repository for scikit-learn.
BSD 3-Clause "New" or "Revised" License
0 stars 2 forks source link

post-link script fails in win-32 present? #12

Closed tomashek closed 2 years ago

tomashek commented 2 years ago

https://github.com/AnacondaRecipes/scikit-learn-feedstock/blob/30d8258de14e263ef07323fa4fdeb0671ee7407d/recipe/post-link.bat#L7

According to this line, the script exits 1 if win-64 does not exist. I think that is the wrong result, as scikit-learn should still install without error. According to my reading of the comments in the script, it should just NOT show the notice about sklearnex if on a 32-bit system. The install should still succeed, so it should exit(0).

Correct?

We have reports of errors on a user system where 32-bit miniconda is installed. Such a scenarios means that the post-link script fails, and conda then fails the scikit-learn install because it is exiting non-zero.

tomashek commented 2 years ago

@remkade, @adipietro18 for awareness

xaleryb commented 2 years ago

@remkade, @adipietro18 if you're guys agree with Todd's comment, please correct this asap because this problem is real blocker for us now. This fix looks really simple.

remkade commented 2 years ago

We're reviewing the fix in the other PR currently. Should be able to get this merged pretty quick.

antkneed commented 2 years ago

An updated build of scikit-learn to address this should be available in defaults.

xaleryb commented 2 years ago

big thanks for doing quick update. Could you please provide link for new build which includes fix?

antkneed commented 2 years ago

It would be this build:

$ conda search --subdir win-32 scikit-learn=1.0.2=*1
Loading channels: done
# Name                       Version           Build  Channel
scikit-learn                   1.0.2  py37hcf78d83_1  pkgs/main
scikit-learn                   1.0.2  py38hcf78d83_1  pkgs/main
scikit-learn                   1.0.2  py39hcf78d83_1  pkgs/main

Direct links for win-32 would be:

xaleryb commented 2 years ago

Thank you. Just for my personal understanding is it possible to prepare the same fix for 0.24.2 version of scikit-learn? Or you are usually doing fixes only for latest version?

antkneed commented 2 years ago

For win-32, this build of scikit-learn v0.24.2 should already have the same fix:

$conda search --subdir win-32 scikit-learn=0.24.2=*2
Loading channels: done
# Name                       Version           Build  Channel
scikit-learn                  0.24.2  py37hcf78d83_2  pkgs/main
scikit-learn                  0.24.2  py38hcf78d83_2  pkgs/main
scikit-learn                  0.24.2  py39hcf78d83_2  pkgs/main

If it does not, please let us know. This is the origin of the fix we applied today. It looks like it was not ported forward. 😅

tomashek commented 2 years ago

@adipietro18 we actually need the win-64 packages. It was the presence on a user system of a separate 32-bit install that caused the problem.

antkneed commented 2 years ago

scikit-learn v1.0.2 build 1 was uploaded earlier with the fix:

$ conda search --subdir win-64 scikit-learn=1.0.2=*1
Loading channels: done
# Name                       Version           Build  Channel
scikit-learn                   1.0.2  py37hf11a4ad_1  pkgs/main
scikit-learn                   1.0.2  py38hf11a4ad_1  pkgs/main
scikit-learn                   1.0.2  py39hf11a4ad_1  pkgs/main

scikit-learn v0.24.2 build 2 should now be available with the fix as well:

conda search --subdir win-64 scikit-learn=0.24.2=*2
Loading channels: done
# Name                       Version           Build  Channel
scikit-learn                  0.24.2  py37hf11a4ad_2  pkgs/main
scikit-learn                  0.24.2  py38hf11a4ad_2  pkgs/main
scikit-learn                  0.24.2  py39hf11a4ad_2  pkgs/main