Skip to content

Should we continue to support compiler=intelem? #24525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
thomasjpfan opened this issue Sep 27, 2022 · 9 comments
Closed

Should we continue to support compiler=intelem? #24525

thomasjpfan opened this issue Sep 27, 2022 · 9 comments

Comments

@thomasjpfan
Copy link
Member

thomasjpfan commented Sep 27, 2022

I have an build refactor removing distutils and numpy.disutils and only uses setuptools that successfully builds our wheels and passes tests. I think it is best to move to a pure setuptools implementation first, because there are still some lingering issues meson. For example, no editable wheels with meson: https://github.com/FFY00/meson-python/issues/47.

setuptools does not make it easy to support custom compilers: pypa/setuptools#2806. It is doable, but it requires more complexity. (I have not got the intelem compiler fully working on our CI yet, but it is working locally in Docker).

Furthermore, I think our Intel ICC job is not checking the Intel build correctly. Specifically, the following compiles the code correctly with icc:

python setup.py build_ext --compiler=intelem -i build_clib --compiler=intelem

But a few lines later, we run:

python setup.py develop

which overrides the icc compiled extensions with gcc compiled extensions. One can see that gcc is used in the build logs.

Also in the logs we see that icc itself is going to be deprecated:

icc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.

Should we support continue to support and build with the Intel compiler?

CC @jeremiedbb @ogrisel

@jeremiedbb
Copy link
Member

jeremiedbb commented Sep 27, 2022

Specifying --compiler=intelem is only possible using numpy.disutils (https://github.com/numpy/numpy/blob/main/numpy/distutils/intelccompiler.py). Since numpy.distutils is deprecated I guess it's natural that we stop supporting it as well.

To the question of supporting custom compilers, I'd be fine with making it simple at first and then see if we can add support later once our new build system is stable (and if we still find that there's a need for that)

Edit: I think it will still be possible to select the compiler through environment variables CC, CXX, ...

@jeremiedbb
Copy link
Member

I have an build refactor removing distutils and numpy.disutils and only uses setuptools that successfully builds our wheels and passes tests

That's great !

@adrinjalali
Copy link
Member

Been worried about the whole distutils issue. I think this makes sense. Thanks for the work @thomasjpfan

@adam2392
Copy link
Member

Adding to the meson-related discussion here, I was recently playing with the editable installs (mesonbuild/meson-python#47) with meson and meson-python and it is nice.

Some thoughts: I've also found using meson to be quite useful and fast in developing a scikit-learn compatible package (https://github.com/neurodata/scikit-tree). Some cons are that using meson needs some getting used to, and there are some weird things you can't easily do (like run Python in the same root directory). But one can use spin (https://github.com/scientific-python/spin) to make the developer exp easier. I will say the meson.build files are also way easier to read and parse and understand compared to mega setup.py files.

@thomasjpfan
Copy link
Member Author

thomasjpfan commented Apr 26, 2023

With the recent inclusion of executable examples in the browser using JupyterLite in #25887, I prefer for meson to work with Pyodide before switching over. Currently there is work to get SciPy to build with Pyodide here: pyodide/pyodide#3649

@rth
Copy link
Member

rth commented Jun 22, 2023

Yes, @lesteve was looking into building scikit-image with meson for Pyodide in pyodide/pyodide#3874 which would be more comparable to scikit-learn build-wise. Scipy is another story.

@lesteve
Copy link
Member

lesteve commented Jun 27, 2024

I am going to close this one. There does not seem there has any demand for this, if this changes we can have a closer look.

I guess if someone has any interest with support Intel compilers with the Meson build, my advise would be to create a separate issue. Setting CC et al would be the first option. There may also be improvements to our meson.build files. There is a scipy issue about supporting Intel compilers that may be have some useful info, see scipy/scipy#17075.

@lesteve lesteve closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2024
@rgommers
Copy link
Contributor

Note that Spack has CI pipelines that exercise this. It should work, and otherwise there'll be a report at some point.

@lesteve
Copy link
Member

lesteve commented Jun 27, 2024

Note that Spack has CI pipelines that exercise this. It should work, and otherwise there'll be a report at some point.

OK great to hear, it seems indeed there is some kind of scikit-learn Spack recipe: https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/py-scikit-learn/package.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants