-
Notifications
You must be signed in to change notification settings - Fork 47
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
Updating a simple package from numpy.distutils #761
Comments
The most important debugging command, IMO:
Nothing has been installed into the wheel. Two fixes: first, for the auto-copy scikit-build-core can do for Python files, you should either match the project name and the package name, or specify the package name explicitly (just like you would if using hatchling). So adding this: [tool.scikit-build]
wheel.exclude = ["*.f90"]
wheel.packages = ["src/pyscf"] You'll have the following new lines:
Okay, now for the built part, you need to install to the output target dir in the wheel, not the source dir. --- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,7 +8,7 @@ find_package(
# F2PY headers
execute_process(
- COMMAND "${PYTHON_EXECUTABLE}" -c
+ COMMAND "${Python_EXECUTABLE}" -c
"import numpy.f2py; print(numpy.f2py.get_include())"
OUTPUT_VARIABLE F2PY_INCLUDE_DIR
OUTPUT_STRIP_TRAILING_WHITESPACE)
@@ -29,4 +29,4 @@ python_add_library(ac0_lib MODULE "${CMAKE_CURRENT_BINARY_DIR}/ac0_libmodule.c"
"${CMAKE_CURRENT_SOURCE_DIR}/src/pyscf/cas_ac0/accas_lib.f90" WITH_SOABI)
target_link_libraries(ac0_lib PRIVATE fortranobject)
-install(TARGETS ac0_lib DESTINATION "${CMAKE_CURRENT_SOURCE_DIR}/src/pyscf/cas_ac0")
+install(TARGETS ac0_lib DESTINATION pyscf/cas_ac0) (The PYTHON -> Python fix isn't required, as scikit-build-core does set both, but Python is the correct one for FindPython, which you are using. If you wanted to set this up to run directly without scikit-build-core, such as for an IDE, you'd need the correct one) That adds this line:
Don't worry, editable installs are still supported. AFAIU, that's what you want? |
Thanks Henry! This is almost working. Install goes well, and the import paths resolve correctly (no more
when I try to import Also, editable install were not required, but it's a nice bonus! |
Weird that it is a Not sure why this is not more common with linking to numpy though. |
Thanks for the input @LecrisUT. This may muddy the waters a bit, but I am trying installation + tests on a PR here to see how it fairs on different machines (only linux and macos are of concern here). On On |
Pretty sure there is a missing link to a main lib produced or stored by NumPy. To start, I'm using macOS is probably tricker, I think the issue there is it's mixing toolchains. In general, macOS Fortran is tricky due to llvm being the native compiler there. Will try to look into that later. |
Why is it not breaking on the linking stage, and why is thr example working 🤔
|
It’s not breaking on the linking stage because you always leave undefined symbols enabled when compiling an extension module. But why the example is working with this missing, that I don’t know. |
Ahh, this file is now always generated since 1.22.4:
From https://numpy.org/doc/stable/f2py/buildtools/meson.html#automating-wrapper-generation But I'm guessing our simple example doesn't need it, so it's basically empty (and before 1.22.4, wasn't generated at all). |
Fixing an issue seen in #761. Our simple example doesn't seem to catch the missing file, but it's clearly missing. Signed-off-by: Henry Schreiner <[email protected]>
Ah, maybe it's the usage of from pyscf.cas_ac0 import accas Gives error
Otherwise the current example has no functionality that would call |
Adding "${CMAKE_CURRENT_BINARY_DIR}/ac0_lib-f2pywrappers2.f90" to the from pyscf.cas_ac0 import accas locally without any issues, and the linux workflow runs successfully. As expected, no changes on the macos runner though. |
I've managed to make some progress on this by being selective about compiler. Here's my successful attempt: https://github.com/CQCL/pyscf-ac0/actions/runs/9665312105/job/26662083580 As before, the ubuntu installation is happy, and works with gcc 11.4.0 for both the C and Fortran compilers. For macos, I use the default clang on the runner: AppleClang 15.0.0.15000040, as the C compiler, and gcc 13.3.0 as the fortran compiler. This combination appears to work, resolving the installation errors I saw here, when I was using gcc 13 for both C and Fortran. Though I need to do a bit more testing in the coming days by building wheels for distribution. |
I have been experimenting with building wheels for distribution using cibuildwheel on this PR. I've gotten over most of the hurdles, except when I install one of my macos wheels locally and try to import, I get:
I can resolve this with
while the macos wheels are missing these:
Is this a mistake I've made at the cibw level, or in my build system? |
This is what the macOS wheel repairing does - it seems you are disabling it with |
Aha, that was a relic from an old cibw workflow I copied over which I never questioned... but removing it seems to solve the problem! I can now install the wheel and import without issue 🎉 One remaining thing I would like to ask: for the macos wheel the
where the linux wheel did (see above). Why is this? And is it anything to be concerned about? |
Is it using accelerate, perhaps? Does it work and is it reasonably performant? |
Tests run as expected and are comparable in speed (maybe even a little faster) to the previous macos wheel (the one without the libs dir). So, yes |
As for whether it's using accelerate, I don't know. I'm not sure what that is. If you're referring to this, then probably not. I'm testing in a completely fresh environment |
hmm, now I'm definitely not sure. I don't know how I would confirm whether it's being used. I've never actively installed accelerate myself |
I've got a start here: https://github.com/scikit-build/f2py-cmake. I don't have a way to generate sig files first, like in yours (started from scratch). Feel free to advise, and maybe if you could suggest a more complex example that would illustrate building it in parts. I don't know much Fortran. :) |
Neither do I to be honest 😅 @mkrompiec is the brains behind the fortran part, I've just taken on the challenge of trying to turn it into a modern looking python package and get it on pypi. I'll keep an I eye that new repo though. As far as this issue is concerned, my project is building and wheels seem to be working, so I am happy for it to be closed 😄 Thanks again for all of your help, I've learned a lot! |
I just hacked, refactored and wrapped portions of GammCor. All I’ve done is a half-automatic translation from F77 to F90, pruning unused pieces of code and making it work with f2py. |
Hi all, I am looking for a bit of guidance in moving a simple package from numpy.distutils to scikit-build-core, following the example here.
My repo is here: https://github.com/CQCL/pyscf-ac0/tree/trying_scikit_build_core
I have a slightly more complex project structure than the example, in that I have a couple of python files interacting with a fortran90 lib (
src/pyscf/cas_ac0/accas_lib.f90
), I have a project executable (rdm_ac0
), and it is intended to be an extension to pyscf, so the code is nested a few directories deep. I'm also a cmake novice, so I'm having trouble bringing it all together.Ultimately, after
pip install .
and running the project executablerdm_ac0
, I get:similarly, if I open a python kernel and try to import, I get the same error
Any help is appreciated, thanks all
The text was updated successfully, but these errors were encountered: