-
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
Experimental support for statically linked NOMAD #126
Experimental support for statically linked NOMAD #126
Conversation
Early experimental support for statically linked NOMAD libraries, NOMAD executable and PyNomad interface. Made changes to cmake targets, updated examples and standard PyNomad to match.
Welp. This did not go as planned. Time to boot a Windows machine up. |
Let me know if I can help. I have access to a Windows/Linux/OSX machine. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a single nomad static library (nomad.a or libnomad.a) we can keep the name nomadUtils, nomadEval and nomadAlgo for dynamic library.
A flag to have CMake (-DStaticBuild=ON) produce the static library or not could be convenient.
Usually, we first do PR to the Develop branch. Once testing and merging in Develop works fine we do a PR Develop->Master.
Thank you for the offer. I'll first look into what went wrong before having you waste time on a wild goose chase. Just finished setting up MSYS+MINGW on Windows, built the PR and failed with identical error as the CI. I am now in position to investigate.
This could be a good way to go. It should make the proof of concept a lot more straightforward: essentially just replace
I understand. (I'll change the merge target once I get MSVC to build.) Like I said in the beginning, I made this PR mostly as a demonstration that static build could be done, especially in the context of PyNomad. I expected there would be hiccups on other platforms, although frankly I hoped it would be much smoother. :) |
Triggered with -DWITH_STATIC_BUILD=ON.
It seems to be getting into shape. I have not realized MSVC was so finicky about function declarations, but then again MS had to bring their own calling conventions so I should have expected there would be trouble. For some reason FreeBSD build fails even before it starts due to missing package manager. Overall changes
Following your comment I'll merge the |
Overall, - added PyNomadStatic cmake taget, - improved PyNomadStatic setup and added proper support for MSVC.
Current iteration.
|
@ctribes Just checking in. Current state (brief overview)
I believe the current version shows static builds are possible even though the current build process could be improved upon. I have verified its functionality on Linux (gcc), on Windows (mingw) and Windows (msvc) builds. I have also checked that the statically linked variant of Questions and notes about possible directions of development
I would love to hear your thoughts on this whole endeavor. 1 One can always use |
Works on OSX. Need to verify on linux and windows.
@jan-provaznik, I ran some tests on MacOSX (x86_64). It required some minor modifications.
I think that building both dynamic and static libraries by default is a good idea. The nomad binary and interfaces will be more portable. The library mode examples will work as long as the user do not move/erase the shared object library.
I did such modifications on my side. Still need to test on windows. I could contribute whenever we have a branch for this in bbopt/nomad.
The modifications are not too big and it really makes PyNomad (probably other interfaces too) building more simple. |
Thank you for your feedback.
I am quite happy to contribute. A lot of my own work has been achieved with NOMAD and it feels only appropriate to give something back in return. Could you please share the necessary modifications? While I'll probably get my hands on some OSX virtual machine eventually, this could speed things up.
Yup.
On this note, perhaps
Alright. I have probably cooked up something similar. We'll get there.
Thanks for the outline. I'll get to it. Should I make these (steps) as separate PRs against the devel branch? Or would another approach be preferable for you? Thanks! |
@jan-provaznik. We can have a branch and a single PR against the development branch for everything that is already in jan-provaznik:feature-experimental-static (steps 1 and 2). We can do other PRs for the remaining steps. I let you create this branch and the PR. I will transfer what I have done (if necessary) when it is available. Without the WITH_STATIC_BUILD option, current Cl actions will build both types of libraries and we will see what breaks.
I got used to -Wl,-rpath. But if what you suggest can bring flexibility, I vote for it. Once available, I will try it to see how it works for OSX in particular. |
Build both static and shared libraries. Link C interface statically. Link Python interface statically. Link nomad executable statically.
Update incorporating the first two steps.
Tested manually on Linux (GCC) & Windows (both MSVC and MINGW). Second commit fixes my mistake which accidentally removed |
@jan-provaznik, I cannot contribute to your branch jan-provaznik:feature-experimental-static. |
I am not sure. The C-Interface is used by the Julia interface. This is done apart from bbopt-nomad. I asked the people if it is convenient or not. |
I'll figure out how to give you access (probably by adding you as a contributor I guess).
Alright. I'll leave it in for now. Please let me know how they respond. PS: Do you know why the M1 and BSD builds fail occasionally with missing |
They prefer to keep the C-interface linked to dynamic libraries for now. |
Works on OSX. Need to verify on linux and windows. Get Nomad version from nomad_version.hpp. Only PyNomad static build.
@jan-provaznik I still cannot contribute to the forked repo. It is too early to merge the PR into the master branch. I am not super familiar with forks. Any idea what is the best approach ? |
Changed the relevant CMakeFile.
I am sorry. You should be a contributor now. You should be able to do whatever you desire with the fork.
Truth be told, I have no idea. I am aware NOMAD has a private development branch, the public repository is essentially an official fork of it. I suppose if this experimental feature proves to work well and there are no apparent regressions we can clean it up and leave it waiting around until the the right time for merge (next release window?) comes. In the meantime I'll move to the next steps we've outlined earlier. I'll do that in a different branch forked from the cleaned up version of this one and we'll see how well that goes. |
…provaznik/bbopt-nomad into feature-experimental-static
@jan-provaznik I am now able to contribute to your forked repo. Thanks. I will work on the Cl for Actions to build the Python interface. When this is done and the tests pass. I will merge the PR to the develop branch and remove the PR to the master branch. |
You are welcome.
Thanks.
Thanks for testing. I've only had a chance to test with python 3.10 (both msvc and mingw).
Alright. Glad to hear! I've started the necessary work to make automated wheel construction work. I'll keep you posted. |
Huzzah! @ctribes I've managed to put together a working wheel-building action over at feature-pypi on top of the latest feature-experimental-static branch. The workflow builds binary wheels for different versions of Python targeted for various Linux platforms, for OS X and for Windows. These are then automatically uploaded to PyPi. I am currently using its testing repository for development. It is possible to test these pre-built binary wheels with
I've done some manual testing on Windows 10 and Linux, but alsas I do not have OS X available. The prototype workflow is triggered manually. The final version should be triggered by release of new version. |
Great work @jan-provaznik. I tested the pip install on my OS X and PyNomad is working. |
Thanks @ctribes. Thanks for testing on OS X. The approach I took in my workflow was chosen out of sheer necessity. There are several constraints one has deal with, namely
Python community decided to handle the second constraint by defining some compatibility-levels which can be used to target as many distributions as possible. This is where manylinux images come to play. One builds their wheels on these systems, does some black magic and hopes for the best. It is where the cibuildwheel package shines: it makes it easy again by automating builds on different manylinux images. The workflow uses this tool. It builds only the Even without the first constraint, it would make no sense to publish PyPi packages on every run of the CI pipeline. Of course, it makes sense to build and test PyNomad (and other interfaces) in the CI pipeline. On this note: currently it is not possible to build PyNomad with OpenMP active. Whether this is an artificial constraint or not (*) is to be determined, however, the CI pipeline should test both combinations: one where OpenMP is active (and PyNomad is not built) and one where OpenMP is disabled and PyNomad (or the MATLAB interface) is built. I'll improve the proposed PyPi workflow to include tests. Fortunately cibuildwheel supports testing out of the box; it should only be a matter of writing appropriate tests. (*) I suspect it is artificial — to prevent PyNomad users from shooting themselves in their feet. I've been using OpenMP enabled NOMAD in nomadlad without any issues, that is, as long as |
As I am a novice in PyPi, this is very valuable information. I am so glad you work on this!
PyNomad is supposed to work with OpenMP enabled. But on Windows (if I recall correctly) I had some weird crashes. I decided to let it go and work on it later. I will add a new issue about that to investigate what is going on. |
You are welcome. 👍 I've added some unit tests in the feature-pypi branch. These are executed automatically against each binary wheel built. The action is still running but it looks promising. I suppose the whole thing is becoming ready to be merged into feature-experimental-static.
I see. Good luck with your investigation! |
@ctribes A quick overview of the changes.
|
@ctribes Is there anything else I can do in this direction? I suppose some README files need to be updated. There is one point which I am not sure of and would like to hear your opinion on.
A final note, if #132 proves stable, we'll probably have to modify setup.py to link against |
@jan-provaznik I believe updating the README files and the tests configuration are the two things remaining. For the PyPi package only the "HOW TO USE" section from the current readme.txt can be reused. Also, a link to User Guide is needed. I could migrate the content of the current readme.txt into a new section of the main README.txt. It would be better I think. I can take care of these changes. I will probably also do the same for the readme of Matlab and Java interface.
For me this is rather equivalent. I will follow your judgement on that.
|
Modifs applied to branch feature/release44 |
Early experimental support for statically linked NOMAD libraries, NOMAD executable and PyNomad interface.
Disclaimer
Overview
CMakeFiles
for examples to match the new cmake targets.CMakeFiles
for PyNomad to match the new cmake targets as well.Statically linked PyNomad
I have added a brand new setup script (setup_PyNomadStatic.py) for statically linked PyNomad interface. Its current version only supports Linux. I'll test (and extend) the script on Windows. Currently one can execute the following to build the binary distribution wheel
where the exported
NOMAD_INSTALL
variable should point to the place NOMAD was installed to (throughcmake --install
) andNOMAD_VERSION
is arbitrary version string which should eventually match the release of NOMAD.Please note that the
build
directory must be removed before running the setup setup script, especially if the interface was built throughcmake -DBUILD_INTERFACE_PYTHON=ON ...
. Both the static and the dynamic version of the package share identical name (i.e., PyNomad) and this confusessetuptools
if one or the other version already exists.