Cannot pip download source
ccoulombe opened this issue · 15 comments
Describe the bug
The setup.py
requires an empty site.cfg
. When running pip download phono3py
, it crashes since there's no config file.
Line 8 in 6c4c561
To Reproduce
$ pip download phono3py==2.3.0
Collecting phono3py==2.3.0
Installing build dependencies ... done
Getting requirements to build wheel ... error
ERROR: Command errored out with exit status 1:
Solutions
- Remove check for an empty
site.cfg
- Provide an empty
site.cfg
It is needed a way to tell users to avoid using single-threaded phono3py.
But I never considered your case. I will follow
Remove check for an empty
site.cfg
Do you want to the release as soon as possible?
Would providing a site.cfg
with openmp
works? Or adding the extra cflag in the setup.
I built the wheels using the sources from Github so I do not need the release as soon as possible, thanks though!
There are several flags of openmp for different compilers and environments.
From the point in PR #60, no default setting is expected. But empty site.cfg
results in phono3py without openmp, which is too slow to use. So I added the check in setup.py
. But as you suggested, raising exception in setup.py
is clearly a bad practice. What I am considering now is in run time to warn if single thread version phono3py is found.
Thanks for your advice.
Suggestion as seen in many other setup.py
. The OpenMP flag could be provided by default for the main compilers and their platform, with the flexibility to define or override with a site.cfg
file.
For instance, on Linux platform with gcc the flag is -fopenmp
. But if one provide a site.cfg
, then use it.
(As long as this is clearly documented in the build/install instructions.)
Circling back to my previous suggestion, this could be simpler by providing by default a site.cfg
with OpenMP assuming the compiler is GCC such as mentioned in the raised exception from the setup.py
. Commented into the file could be the other varian such as for clang, etc.
As long as all this is documented!
# This assume that the compiler is GCC
"[phono3py]",
"extra_compile_args = -fopenmp",
My only objection would be that a default site.cfg
will have the same problem when people are using compilers that don't support the -fopenmp
flag. NAG for instance (I don't know if they have made a redirect yet).
Another route would be to default the -fopenmp
flag, and let another command line option redefine the openmp flag, that can still be done using pip.
I can see that @atztogo changed it to fail if site.cfg
was not present. That was not part of my original re-implementation.
My suggestion is to keep things simple since the preference is to build with OpenMP by default. As long as it is documented, it is not an issue (imho) to edit a configuration file to suit the current compiler/platform.
As with Numpy site.cfg
, one can still provide the different configuration but commented and document how to use it.
I can see that @atztogo changed it to fail if site.cfg was not present.
If someone automates the deployment of phono3py, their system could produce phono3py without openmp, but the maintainer might not realize the fact (most probably their tests pass). This situation is expected to be avoided. So some way to inform this transition of compilation way was needed. Documentation may be read when people find the problem, but the problem is silent, it is not easy to lead people to the documentation.
Probably we have to give up site.cfg
approach due to deprecation of numpy.distutils.
In order to keep things simple: adding the flag to the setup, you can output a clear message to the user that the flag was added and directing him to the documentation.
@ccoulombe, thank for your comment. What do you mean about "the flag"? Could you explain a bit more detail?
The flag we are discussing is -fopenmp
or its equivalent. What I am suggesting is doing as Pytorch: https://github.com/pytorch/pytorch/blob/937ca69f15f8f1c8516c7d4cd86bdcaeb765a23c/setup.py#L758
In order to force the compilation with openmp by default, for various platforms
Something like :
if is windows:
extra_compile_flags += /openmp
else:
extra_compile_flags += -fopenmp
And you can add a map of compilers and their respective flag : {'gcc': '-fopenmp', 'clang': '-fopenmp', 'icc': '-openmp'}
But with the deprecation of distutils, it might be a very good opportunity (and cleaner) to switch to Meson and ask it for OpenMP
https://mesonbuild.com/Dependencies.html#openmp
Another option, could be user options as pyFAI does: https://github.com/silx-kit/pyFAI/blob/819aa277c637d13388a72f4e513367bd87279e46/setup.py#L482
You could even combine both (option and meson) as scipy has done:
https://github.com/scipy/scipy/blob/main/meson.build
https://github.com/scipy/scipy/blob/main/meson_options.txt
Thanks @ccoulombe for detailed information that helps me a lot. I will consider how we should do among those options.
I could find some time and energy to challenge this issue. The comments above were very helpful and useful to start since this kind of information is fairly limited. The following is just for my memorization of my attempt.
As the initial attempt, I employed cmake rather than meson. The reason I didn't use meson was just that I am a little bit familiar with cmake. If this approach works, I consider (hope) that it will be easy to move to meson.
Automation and customization are needed in the build process under our many constraints. This attempt was/is the former. Still it is under the development. The latter will be next.
Basic idea
- Cmake is called in
setup.py
using subprocess. The temporary working directory is_build
. - Codes in C are made as static-link libraries that are compiled using cmake.
- Python-C interfaces (
c/_phono3py.c
,c/_phononmod.c
) are complied by setuptools that is invoked insetup.py
and the libraries made by cmake are linked with them.
Use of find_package
in cmake for finding external libraries
- OpenMP, BLAS, and LAPACK libraries and their required compile-flags are searched by using
find_package
. - Information found are sent to stdout (using
message
of cmake) andsubprocess
receives them as strings. - The information from cmake is used for linking the external libraries by setuptools.
Things to do
- Currently no means of customization by users such as proposed by @ccoulombe just above.