Ambertools 24 - #162
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe/meta.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/13529973597. Examine the logs at this URL for more detail. |
|
|
AFAICT that's a simple typo, not a deprecation or breaking change between versions |
|
Yah I will patch it -- I'll see if it still exists upstream and contribute it back as well |
|
@dacase Do you know what versions of python ambertools 24 should support? On linux python 3.10 compiles fine. I am running into this error in a python 3.11 linux build: And this error on a python 3.12 linux build: Looking at the source, both errors are in some |
|
(also crazy thing @mattwthompson -- that typo doesn't show up in the source code, I checked both patched and unpatched versions, going to try building but this is a weird one) EDIT -- locally actually I might have rc5 instead of rc6 -- checking that now |
|
Ah I was pulling in the wrong source (explains the weirdness + why some patches worked that I expected to have to fix) |
|
Okay getting this error for the linux In the recipe, we have: And this is how it gets rendered: So not sure why it can't find mpi (or why this worked with ambertools 23) And the mpich package has it, https://conda-metadata-app.streamlit.app/Search_by_file_path?path=include%2Fmpi.h but 1.0.1 is kinda old I think? Just filing this away as one error mode, depending on how this shakes out we can drop the MPI builds until someone else has time to work on them -- any ideas @njzjz ? |
|
Same error with the openmpi builds pulling in |
|
osx-x86_64 openmpi and mpich are also failing with the same issue |
|
Predictibly, the osx-arm64 builds are also failing for openmpi and mpich |
|
Cool so all the nompi builds worked, going to test them now... |
|
On Mon, Feb 24, 2025, Mike Henry wrote:
@dacase Do you know what versions of python ambertools 24 should support?
On linux python 3.10 compiles fine.
I am running into this error in a python 3.11 linux build:
```
pytraj/trajectory/frame.cpp:35169:1: error: too many initializers for 'PyTypeObject' {aka '_typeobject'}
```
And this error on a python 3.12 linux build:
```
pytraj/trajectory/frame.cpp:38721:24: error: 'PyThreadState' {aka 'struct _ts'} has no member named 'curexc_type'
```
Looking at the source, both errors are in some `PY_MAJOR_VERSION` if blocks -- but there doesn't seem to be a diff that would impact this around those lines but the top of that files does have some changes to the cython abi version.
--
Reply to this email directly or view it on GitHub:
#162 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
Mike:
I'm cc-ing this to Hai Nguyen ***@***.***>, who is still active as
the author of pytraj. Personally, I never see any linux build errors that
depend on the python version, up to and included 3.13.
Is it possible that what you report above depends on both the g++ version
and the python version? Or maybe it's the cython version that is giving
problems.
Most recent pytraj errors are coming from OSX, and I don't know if
anyone has a working version of pytraj + cpptraj on osx-arm64. But
Linux has been fairly trouble-free. The "ambertools-dac" version
24 conda package seems to have a functioning pytraj on linux: see
https://ambermd.org/pmwiki/pmwiki.php/Main/Amber24Test.
Long term (or maybe short-term), I would love to split off pytraj/cpptraj
into a separate conda-forge package. The cpptraj part looks ready to go,
but Hai and others seem to be still struggling with pytraj. See
https://github.com/Amber-MD/pytraj/issues for some recent posts.
Sorry I can't be of more direct help.
...dave
|
|
@dacase -- no worries! I was able to get it to build okay, the issues were my fault (I had mixed up some sources) We should be good to release ambertools 24 now (skipping CUDA and MPI builds for now) for osx-arm64, osx-x86_64, and linux. Future work will be to get those other builds working. I'll also give numpy 2 a try after we get the first round of builds uploaded. |
|
I do want to give a quick go on python 3.13 builds... |
…nda-forge-pinning 2025.02.24.19.33.50
Let me know if you want my help with that part, if only to interpret any apparent failures. |
| - patches/0003-look-in-right-place-for-libcudadevrt.a.patch | ||
| - patches/0004-adapt-CMake-function-calls-that-were-due-to-find_pac.patch | ||
| - patches/0005-set-enable_language-CUDA-in-a-bunch-of-places.patch | ||
| #- patches/0001-do-not-clean-compiler-flags.patch |
There was a problem hiding this comment.
Okay getting this error for the linux
mpichbuildscd $SRC_DIR/build/AmberTools/src/quick/src/octree && $BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DCEW -DGNU -DMPIV -I$SRC_DIR/AmberTools/src/quick/src/octree/. -std=gnu++11 -fPIC -O2 -mtune=native -MD -MT AmberTools/src/quick/src/octree/CMakeFiles/octree_mpi.dir/octree.cpp.o -MF CMakeFiles/octree_mpi.dir/octree.cpp.o.d -o CMakeFiles/octree_mpi.dir/octree.cpp.o -c $SRC_DIR/AmberTools/src/quick/src/octree/octree.cpp In file included from /home/conda/feedstock_root/build_artifacts/ambertools_1740429248536/work/AmberTools/src/quick/src/octree/octree.cpp:18: /home/conda/feedstock_root/build_artifacts/ambertools_1740429248536/work/AmberTools/src/quick/src/octree/octree.h:8:10: fatal error: mpi.h: No such file or directory 8 | #include <mpi.h> | ^~~~~~~ compilation terminated.In the recipe, we have:
- {{ mpi }} # [mpi != 'nompi']And this is how it gets rendered:
- mpi 1.0.1 mpichSo not sure why it can't find mpi (or why this worked with ambertools 23)
And the mpich package has it, https://conda-metadata-app.streamlit.app/Search_by_file_path?path=include%2Fmpi.h but 1.0.1 is kinda old I think?
Just filing this away as one error mode, depending on how this shakes out we can drop the MPI builds until someone else has time to work on them -- any ideas @njzjz ?
I used this patch to fix the QUICK MPI issue. See also merzlab/QUICK#343
There was a problem hiding this comment.
Thanks! Add the builds back and re-enabling that patch (which I removed by mistake thinking it was CUDA related)
|
@dacase Getting this error when building python 3.13 on linux and osx-arm64 -- not sure if this is a python 3.13 change for This also isn't blocking to getting ambertools 24 released today, I can fix this issue in a follow up PR. |
…nda-forge-pinning 2025.02.25.13.14.53
|
RE: Python 3.13 builds: It looks like it was added as a public method, but it doesn't look like it is a simple find and replace since the return signature has changed. |
|
Punting on 3.13 seems like an easy call to me (as does giving it a college try beforehand!) |
@mikemhenry Hi, per the another report in here, Amber-MD/pytraj#1677, recythonize the pyx files with python 3.13 would help. Cheers. |
|
@hainm I was just working on that but I am running into a lot of issues doing that using the setup.py that ships with the ambertools, I will try using the setup.py from the repo using version |
Can you please have a look at |
|
@hainm -- where is that file located? I can't seem to find it in https://ambermd.org/downloads/AmberTools24_rc5.tar.bz2 |
oh right. It's not there, it's only in git version. |
|
Ah I do have access to the git version, didn't think to check there, thanks! |
|
Testing the cythonizing fix now locally -- I think we will need to do a boost migration to get python 3.13 builds on osx-64 Can't solve for rdkit |
…nda-forge-pinning 2025.02.25.13.14.53
|
This is using 1.84 which can't be that old, right? |
|
Let me check, it might, and looking at the error there could just be a hole in rdkit builds for intel macs and python 3.13 |
|
Yes it is using 1.84 -- so the issue then I believe is a missing rdkit build (don't tempt me with a good time 😄 ) |
h-vetinari
left a comment
There was a problem hiding this comment.
I think the patch for 3.13 support here is an overreach. We should wait for upstream to become compatible with 3.13, not carry such monster patches.
There was a problem hiding this comment.
A patch with 1.1 million lines is... extreme, no matter how mechanical it might be.
There was a problem hiding this comment.
On the other hand, if this is fully generated code by cython, then the solution is simply to delete whatever's checked in (or better: remove it from ambertools upstream), and make the cythonize call a part of the build.
There was a problem hiding this comment.
On the other hand, if this is fully generated code by cython, then the solution is simply to delete whatever's checked in (or better: remove it from ambertools upstream), and make the
cythonizecall a part of the build.
yeah, this is a better plan. Currently, pytraj cythonize doesn't work with cython >= 3.0 (the code causes lots of segmentation fault).
There was a problem hiding this comment.
@h-vetinari I didn't want to make it part of the build since I wasn't sure if it would break older python versions/cause issues, so I only wanted to apply to the python 3.13 builds, so instead of a patch I would have had to add a check on the build "if python 3.13 run cython" but then depending on how it handled updates to cython, it could make a migration fail, not because of the migration but because an update in cython broke things and I didn't want to deal with troubleshooting it.
That is why I settled on a patch. RE: upstream support, I didn't want a "sub" package of ambertools to hold everything back. In a perfect world things would build separately but there is a mix of packages that ought to be vendored (like packmol) and things that could be seperate (pytraj) but then are really hard to link correctly to if someone uses this build/feedstock to build amber.
So I didn't want the lack of pytraj support of python 3.13 to hold back programs like antechamber and sqm that work fine with the python 3.13 build.
I could try turning off the pytraj build when we build python 3.13, I am not sure if doing that will cause issues with other packages being unable to build (for awhile I had to patch out nab2c since turning off the build turned off unrelated packages as well).
| #- patches/0001-use-find_package-CUDAToolkit-instead-of-find_package.patch | ||
| #- patches/0002-rely-on-DCMAKE_CUDA_ARCHITECTURES.patch | ||
| #- patches/0003-look-in-right-place-for-libcudadevrt.a.patch | ||
| #- patches/0004-adapt-CMake-function-calls-that-were-due-to-find_pac.patch | ||
| #- patches/0005-set-enable_language-CUDA-in-a-bunch-of-places.patch |
There was a problem hiding this comment.
Are you going to reattempt CUDA 12.x support?
There was a problem hiding this comment.
someday -- I wanted to leave it all as is, the patches need to be rebased to the new source (I also didn't check to see if the new source "just works" with cuda)
Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)