Build HEAD of v2.10 branch, enabling metatensor #5
Conversation
|
Thanks @Luthaf !!! Shall we have an explicit dependence on libtorch in |
|
It would also make sense to customize the label If this contains everything, we might call it |
There is already a transitive dependency on libtorch through metatensor-torch, but I can add an explicit one if you prefer!
Sure, I'll customize the label! |
|
Is there a way to also build py-plumed from this repository? Since I would need a version with plumed/plumed2#1129 included for the tutorial. If not, I can also try to run the simulation using some plumed-enabled LAMMPS build. |
|
Traditionally we didn't, mostly because it was not necessary. But you can add it. I think it makes sense if the python package for the tutorials is different from the official one. Otherwise, it should not be necessary |
|
So I sould copy the code from https://github.com/conda-forge/py-plumed-feedstock/ here and adapt accordingly? |
Luthaf
changed the title
|
Ok, this has been changed to build the HEAD of v2.10 branch, and now includes py-plumed as well. |
It prevents local builds if the macOS SDK are installed in a different location
|
Updated to pull from https://github.com/plumed/plumed2, at commit plumed/plumed2@b0ffcb5 |
| @@ -18,6 +18,10 @@ export CXXFLAGS="${CXXFLAGS//-O2/-O3}" | |||
|
|
|||
| # libraries are explicitly listed here due to --disable-libsearch | |||
| export LIBS="-lboost_serialization -lfftw3 -lgsl -lgslcblas -llapack -lblas -lz $LIBS" | |||
| export LIBS="-lmetatensor_torch -lmetatensor -ltorch -lc10 -ltorch_cpu $LIBS" | |||
There was a problem hiding this comment.
@Luthaf any specific reason for removing libraries here (boost, fftw3, gslm etc)? I suspect they won't be linked
There was a problem hiding this comment.
I did not remove anything, I added another line below. The full set of libraries will be -lmetatensor_torch -lmetatensor -ltorch -lc10 -ltorch_cpu -lboost_serialization -lfftw3 -lgsl -lgslcblas -llapack -lblas -lz, plus whatever was in $LIBS before these two lines
There was a problem hiding this comment.
Sorry I didn't read correctly the script, it's fine as you did
| conda-build -c conda-forge plumed | ||
| conda-build -c conda-forge gromacs | ||
| conda-build -c conda-forge lammps | ||
| conda-build -m .github/conda_build_config.yaml -c conda-forge plumed |
There was a problem hiding this comment.
Useful trick (-m), so that we have them in a single place, thanks!
There was a problem hiding this comment.
Yes! And this is intentionally not in the repository root, to prevent it from being picked on local builds: https://docs.conda.io/projects/conda-build/en/latest/resources/variants.html#conda-build-variant-config-files
py-plumed/meta.yaml
Outdated
| @@ -0,0 +1,52 @@ | |||
| {% set name = "py-plumed" %} | |||
| {% set version = "2.10" %} | |||
There was a problem hiding this comment.
Is there a standard way to make sure the date is baked in the version in some way? What I am thinking about is if a few weeks we release 2.10 it should override this one. Or shall we just increase the build: number below?
There was a problem hiding this comment.
If you look below, the actual version ends up being 2.10.git.<hash>. I'm not sure if this will be treated as an alpha version by conda, but I can check that. If that's the case, the future 2.10 release should have priority over the alpha release!
There was a problem hiding this comment.
Ok, I changed this to have the version be 2.10.dev1 and store the git sha in the build string instead. The whole package is then plumed-2.10.dev1-git.b0ffcb55ad8cc3c71ec5c30a1bd46f68dd33bceb.
This means that when 2.10 is released, it will be used instead of this one, since conda is following PEP440: https://peps.python.org/pep-0440/
macos-13 is the one to use for x86_64
|
CI failed trying to pull binutils on macos (to build lammps & gromacs). Not sure why this is needed though, they don't seems to be used by the build script. |
|
@GiovanniBussi could you start the CI so I know if it is now running or needs more changes? Thanks! |
Creating this as a draft PR for @GiovanniBussi to comment on
This will need plumed/plumed2#1122 and plumed/plumed2#1123 to be merged first, and is currently pulling from a plumed fork with both PR merged. I'll update to use from plumed master later!