-
Notifications
You must be signed in to change notification settings - Fork 71
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
Create binary wheels for Solcore #139
Comments
Hi @dalonsoa, do you think this two remaining task are still relevant? |
Hi @Abelarm , most certainly! It's also the case that the linux builds are created only up to Python 3.8, so there's work to do there to extend the support to Python 3.9 and, possibly, 3.10. |
Perfect! i'll probably do two separate PRs since the linux one is already up and running |
Hi @dalonsoa I have created two PR:
|
Hi @Abelarm, This looks great! I've already accepted the linux one. For the Windows one, have you checked if the binary wheels created do work in a stand-alone, Windows system that do not have a Fortran compiler? I've seen cases where the binary wheels are created but then you still need to install the compiler because some shared libraries are not included in the wheel, which defeats the purpose of having a wheel! I will try to check that myself later in the week. |
Actually I wanted to try but I don't know how to download the wheel from the action itself without the publishing of the wheel, and since I use a Linux for dev I can't build it on my own :( For this reason I created two separate PRs I knew that the windows one was a tricky |
You will need to work on a branch in your own fork of Solcore with a custom workflow that upload the artefact for download rather than publishing it to GitHub, for example using this action https://github.com/marketplace/actions/upload-a-build-artifact . Another option is to upload it to TestPyPI. From either of those two, you can download it and try the wheel in a fresh system. But, if you don't have access to a Windows machine, you cannot test it, anyway. Let me try this at some point this week. In principle, the PR looks good and should work, but let's test it before we merge. |
I would remove ngspice as you commented, since I never build it for windows I went with there was in the test |
I've run a job that uploads the wheels as artefacts. You can find them here: https://github.com/qpv-research-group/solcore5/actions/runs/3221669724 I'll try the windows ones first thing tomorrow morning, but considering their small size, I'm a bit suspicious about they having all the required dependencies. I'll post my findings. |
Great news! I've just tested the Windows wheels on a fresh Windows 11 system with Python 3.10.7 and they work well without installing anything else! This is going to be a huge leap forward for many people!! Many thanks for making it happen! |
YEAH great news!! Happy to help !! I'll add the mac-os as well (but neither that I can test it) |
Completed in #229 , excluding M1 architecture for Apple |
Solcore contains some fortran code - for the Poisson-drfit-diffussion solver - that needs to be compiled and converted into an extension module that can be imported from Python. That is done automatically when installing Solcore using the
--with-pdd
flag, which employs F2Py under the hood, a tool included in NumPy.The caveat is that, for this to work, the user installing Solcore - either from PyPI or directly from GitHub - will need to have a suitable fortran compiler at hand and all the dependencies libraries since this flag disables the use of wheels for any of the Solcore dependencies. All of them will need to be recompiled. It is not a huge deal for Linux and MacOS users, but it is a pain for Windows users.
The solution is to distribute binary wheels for the three OS, ensuring that all dependent libraries needed by the fortran code are statically linked with the expansion module. The solution (for each of the OS) should work in GitHub Actions such that, when there is a new release, the wheels can be automatically uploaded to PyPI.
This is an umbrella issue that will be updated with the individual, OS specific issues:
The text was updated successfully, but these errors were encountered: