-
Notifications
You must be signed in to change notification settings - Fork 23
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
experiment with scikit-build-core as the build system #98
Conversation
Hi, thanks for the contribution. Personally I do not want to have the extra dependence on The |
No problem--I understand if you don't think switching is the right choice here. Thanks for replying so quickly -- and thanks for the tip about obtaining distutils via setuptools. Out of curiosity, what kinds of environments required the most changes to CMakeLists.txt? (Maybe it is a cluster with an old CMake version ?) |
Thanks for your understanding!
For example, you can see the GitHub actions failed with your changes on I agree that the old way of finding We are trying hard to remove dependences (or at least making things optional): Python is optional, MKL is optional, MPI is optional, and even Loosely speaking, all complexities in For the most common case, |
I see what you mean. If my previous changes broke your CI, that's quite bad. I tried working in a pure Python venv and I see how some of the mysterious-looking CMake configs were important for finding pybind11, as you pointed out. Thanks for explaining all of this. I understand what you're saying about keeping dependencies optional in CMakeLists.txt so the user has a lot of flexibility in configuration. Sorry for interfering with that! As a proof-of-concept, I restored the original CMakeLists.txt and added only the minimal changes necessary for scikit-build-core to work. I think now the github action should work (fingers crossed!). Actually, the same CMakeLists should work with your original setup.py --- from CMake's perspective, scikit-build-core is completely optional. For my own use, I need to compile against my cluster's MKL and MPI installations, but python 3.11 is totally fine. I will follow the instructions online and refer to CMakeLists.txt to set the proper environment variables, etc. Hopefully there won't be problems and it'll go smoothly. |
For this, |
Good point about the workflow! Rather than retain the setup.py, which may confuse scikit-build-core, I provided a copy of the workflow file with all the sed commands changed appropriately. This was possible since pyproject.toml has nearly all configurations previously in setup.py, just in a different syntax. |
Thanks for the changes. Unfortunately, the For why we need to additionally build this non-Python executable in the package: when you need to run very large scale DMRG (using more than 10k CPU cores), the overhead of Python/numpy will become significant (in certain computer architectures). The only way to get desired scalability is to get rid of Python. So this non-Python version is important. For why it makes sense to include non-Python binaries in a Python package: this is common practice, see https://pypi.org/project/mkl/, https://pypi.org/project/cmake/, etc. In general, as I explained before, if |
Your reasoning certainly makes sense--there's no real reason to switch to scikit-build core since the current build system works fine. I agree with this. If, one day, setuptools+setup.py no longer provides a good solution, then it might be worthwhile. Also, sorry for failing to notice the block2 executable being built and shipped in the wheels. Oops! 😖 Thanks for your patience. Your suggestion about supporting scikit-build-core alongside the usual build system is very interesting. I only know one package that does this -- mpi4py -- so it should be possible! I will play around with this to see whether it can be done without making setup.py too ugly, and I'll get back to you with the results. |
Thanks for your efforts on improving this! |
Supporting multiple build backends like mpi4py seems like it will add complexity and add new sources of bugs. A stripped-down version of the method is to use a custom build backend like this one:
I'm not too enthusiastic about this solution. Maybe it's best to close this PR? |
I am glad that we reached an agreement at least in the context of
which is surprisingly clear and easy for any adaptation, if compared with something like |
I think scikit-build-core is mature enough to rely on these days. There are some rough edges, but it's much nicer to use than distutils, and it takes care of many things automatically. Now that distutils is gone from python 3.12, it makes a lot of sense to migrate.
I've been able to build block2 for python 3.12 with scikit-build-core without that much trouble. Here's what I did. Give it a try and let me know how it goes!