Conda package for cpptraj #1125

dacase · 2025-01-02T15:55:36Z

@drroe This is actually my first attempt at a pull request that is based on a non-master
branch. I'm assuming that accepting the request will create a conda branch at Amber-MD/cpptraj, and not mess with your master branch.

You can also just go to github.com/dacase/cpptraj.git.

…Need to check if this is OK for Linux

…dine.a. Need to check if this is OK for Linux" This reverts commit d81868f.

This reverts commit 0546fe9.

…reak internal version?

… be expected; need to check for breaking internal readline

hainm · 2025-01-03T16:05:29Z

@dacase Do we want to build libcpptraj and include header files in this PR too? or it should be different recipe?

dacase · 2025-01-03T16:14:52Z

We want to make this recipe to the right thing for pytraj. There is a libcpptraj.so file in $CONDA_PREFIX/lib in the conda package. I don't know what headers pytraj needs, or where they should go -- $CONDA_PREFIX/include? Check out the rsync line towards the end of build.sh, and see if that can do what is needed.

hainm · 2025-01-03T16:44:12Z

I don't know what headers pytraj needs, or where they should go -- $CONDA_PREFIX/include?

For now, we could copy all the src/*.h to ${CONDA_PREFIX}/include/cpptraj/ folder
export CPPTRAJ_HEADERDIR=${CONDA_PREFIX}/include/cpptraj

hainm · 2025-01-03T16:45:15Z

There is a libcpptraj.so file in $CONDA_PREFIX/lib in the conda package.

It seems this comes from AmberTools conda since I don't see where libcpptraj.so is built in your recipe. I will make further comment in the code.

hainm · 2025-01-03T16:47:15Z

There is a libcpptraj.so file in $CONDA_PREFIX/lib in the conda package.

It seems this comes from AmberTools conda since I don't see where libcpptraj.so is built in your recipe. I will make further comment in the code.

never mind, you do use "-shared" in your build.sh so libcpptraj.so is already done.

hainm · 2025-01-03T16:48:21Z

recipe/build.sh

+
+
+rsync -a README.md config.h LICENSE dat bin lib test $PREFIX
+


mkdir -p $PREFIX/include/cpptraj rsync src/*.h $PREFIX/include/cpptraj/ # Not sure if this is correct syntax :D

hainm · 2025-01-03T16:49:47Z

recipe/activate.sh

+#!/bin/bash
+
+export CPPTRAJHOME=${CONDA_PREFIX}
+


Also add export CPPTRAJ_HEADERDIR=${CONDA_PREFIX}/include/cpptraj so pytraj can find the header files. Ideally, we should use CPPTRAJHOME/include but the header files might overwrite other library's headers, so we make "cpptraj" folder.

@hainm do a conda install dacase::cpptraj (only for linux-64 right now), and see if libraries and headers seem to be in the proper places with the proper information.

@hainm do a conda install dacase::cpptraj (only for linux-64 right now), and see if libraries and headers seem to be in the proper places with the proper information.

@dacase I confirm that I do see $PREFIX/lib/libcpptraj.so and $PREFIX/include/cpptraj/*.h
Thanks.

@dacase It seems we need to recursively include the .h files too. pytraj only uses a subset of them but one header refers another/other header file(s), so think we need to include all.

drroe · 2025-01-03T22:47:16Z

I've made some progress on fixing external readline support in cpptraj. It will be in a separate branch that can then eventually be merged into this PR, which should fix the test errors.

dacase · 2025-01-03T23:58:34Z

I've made some progress on fixing external readline support in cpptraj. It will be in a separate branch that can then eventually be merged into this PR, which should fix the test errors.

I'm a bit lost here: did the test errors arise from the conda package, or from trying to compile (and if so, from which branch?) Can you say which tests are throwing errors? I've never quite understood how to test for readline problems.

drroe · 2025-01-04T00:14:34Z

I've made some progress on fixing external readline support in cpptraj. It will be in a separate branch that can then eventually be merged into this PR, which should fix the test errors.

I'm a bit lost here: did the test errors arise from the conda package, or from trying to compile (and if so, from which branch?) Can you say which tests are throwing errors? I've never quite understood how to test for readline problems.

See here: https://jenkins.jasonswails.com/blue/organizations/jenkins/amber-github%2Fcpptraj/detail/PR-1125/2/pipeline/33/

This is because there is no system readline so cpptraj tries to build the internal one, but the current build setup can't handle the readline includes being in a readline directory. This branch will handle both system readline and internal readline consistently once I get it merged: https://github.com/drroe/cpptraj/tree/readline.include

…j, for use by pytraj

drroe · 2025-01-08T15:35:52Z

Sorry for the delay on this. I've fixed (I think) cpptraj's use of system readline libraries (#1126), but I'm running into an issue with CMAKE that I'm having trouble solving. I switched the include in ReadLine.cpp to be the proper #include <readline/readline.h> (as documented here) which works fine for the configure-based install because I switched the compiler include flag to be -I., but fails for the CMAKE-based install because I can't figure out how to change the include flag. I though that maybe modifying the following in SetupThirdParty.cmake would do it:

--- a/cmake-cpptraj/SetupThirdParty.cmake
+++ b/cmake-cpptraj/SetupThirdParty.cmake
@@ -449,8 +449,8 @@ if(readline_EXTERNAL)
                LIBRARIES readline::readline
                INCLUDES ${READLINE_INCLUDE_DIR}/readline)
 elseif(readline_INTERNAL)
-
-       list(APPEND 3RDPARTY_SUBDIRS cpptraj/src/readline)
+        message(STATUS "Readline will be internal")
+       list(APPEND 3RDPARTY_SUBDIRS cpptraj/src)

 endif()

But it's still trying to include src/readline instead of just src so I guess there is somewhere else that needs to be changed:

)$ make ReadLine.o
cd /home/droe/Cpptraj/rl.include/build && make  -f src/CMakeFiles/cpptraj.dir/build.make src/CMakeFiles/cpptraj.dir/ReadLine.cpp.o
make[1]: Entering directory '/home/droe/Cpptraj/rl.include/build'
Building CXX object src/CMakeFiles/cpptraj.dir/ReadLine.cpp.o
cd /home/droe/Cpptraj/rl.include/build/src && /usr/bin/g++ -DBINTRAJ -DBUILDTYPE=\"GitHub\" -DC11_SUPPORT -DFFTW_FFT -DHASBZ2 -DHASGZ -DHAS_TNGFILE -DHAVE_FFTWD=1 -DLIBPME -DUSE_MKSTEMP -D_GNU_SOURCE -I/home/droe/Cpptraj/rl.include/src/tng/. -I/home/droe/Cpptraj/rl.include/src/arpack/. -I/home/droe/Cpptraj/rl.include/src/xdrfile/. -I/home/droe/Cpptraj/rl.include/src/readline/. -Wall -Wno-unused-function -Wno-unknown-pragmas -Wno-unused-local-typedefs -Wno-unused-variable -Wno-unused-but-set-variable -std=gnu++11 -O3 -mtune=native  -MD -MT src/CMakeFiles/cpptraj.dir/ReadLine.cpp.o -MF CMakeFiles/cpptraj.dir/ReadLine.cpp.o.d -o CMakeFiles/cpptraj.dir/ReadLine.cpp.o -c /home/droe/Cpptraj/rl.include/src/ReadLine.cpp
/home/droe/Cpptraj/rl.include/src/ReadLine.cpp:11:13: fatal error: readline/readline.h: No such file or directory
   11 | #   include <readline/readline.h>
      |             ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.

It's not clear from the documentation what needs to be modified to ensure the includes are properly handled. @dacase @swails @hainm do any of you have any ideas?

drroe · 2025-01-08T15:48:59Z

I may have figured it out: looks like I had to modify a target_include_directories command in the readline subdirectory itself. Stay tuned...

…#1126) * Fixes for using system readline * Go back to bundled readline if no system readline present * Add checks for testing whether -ltermcap or -lncurses are needed for linking to system readline * Change readline include path for CMAKE * Add the '.' include back for the bundled readline library itself. * Try to fix the clang compile. I thought this was based on the gcc version but maybe not? * Try to upgrade to python 3.11 for pytraj to get CI working again. * Try conda 24.11.0. * Forgot to revert python. I do not like python.

drroe · 2025-01-08T20:58:10Z

The readline issues should be resolved with #1126. Next will be the header files installation.

* Use the install binary * Script to get all headers that should be installed for libcpptraj * Contains all headers that should be installed for libcpptraj * Add target for installing headers * Make header install less verbose. Have libcpptraj install depend on install_headers * Cmake install headers for inside/outside amber * Convert back down to lyx 2.3 for Amber * Update the PDF

drroe · 2025-01-14T17:59:35Z

@dacase Cpptraj header install should be working now for both configure/cmake via #1129. You can try merging the current Amber-MD/Cpptraj master into this branch and see if it works for you.

Alternatively, let me know if you want me to reproduce this PR on my fork if that will be easier.

dacase · 2025-01-14T18:18:20Z

Let me work on this, to make sure the conda package is still OK at my end (i.e. before you do more on this PR at your end. See also coming comments on issue #1120 .

dacase · 2025-01-15T05:46:06Z

@drroe As I mentioned above, I don't think it's worth your while to do anything with this PR per se.

If you find some time (absolutely no rush!), what would be most helpful would be to install the existing conda package, and try out the tests, look at the contents of the include folder, etc.
Only try this on Linux for starters, because the osx builds are a bit behind.

conda create --name cpptraj
conda activate cpptraj
conda install dacase::cpptraj
cd $CONDA_PREFIX
cd test
make test   #  etc.
conda deactivate

(Right now, I've only put the test folder into the conda package; let me know if I should add the unitTests folder as well.)

FYI: the changes in the PR are pretty minimal. Here are the diffs between the conda branch and master on my local machine; (not all of these have been placed yet into the PR):

(cpptraj) bethe% git diff --stat master
 Makefile                       |  4 +-
 configure                      | 17 ++++-----
 mkrelease                      | 14 +++++++
 recipe/activate.sh             |  5 +++
 recipe/build.sh                | 46 ++++++++++++++++++++++
 recipe/conda_build_config.yaml |  8 ++++
 recipe/deactivate.sh           |  5 +++
 recipe/meta.yaml               | 86 ++++++++++++++++++++++++++++++++++++++++++
 src/Matrix.h                   |  1 +
 9 files changed, 175 insertions(+), 11 deletions(-)

dacase and others added 21 commits December 19, 2024 11:55

first complete cpptraj build--still needs packaging work

6c3d897

tweaks to readline for first conda build

0546fe9

tweaks to build.sh: no -openmp, include activate

3110aca

to first version 0.1 on linux

44e4b19

include <algorithm> header, needed for the copy() and fill() methods

bba2b94

defined NEED_EXTERN_PC to prevent duplicate symbols in libreadine.a. …

d81868f

…Need to check if this is OK for Linux

use clang for Darwin, gnu for Linux

ed4f9a6

fix typos in build.sh; add arpack, openmm and openmp libraries

5a0e164

Revert "defined NEED_EXTERN_PC to prevent duplicate symbols in librea…

dbd119e

…dine.a. Need to check if this is OK for Linux" This reverts commit d81868f.

Revert "tweaks to readline for first conda build"

3523e66

This reverts commit 0546fe9.

use -lncurses rather than ltermcap for external readline; will this b…

8d5d4b8

…reak internal version?

look for readline headers in readline subdirectory, as I think should…

40e0ddc

… be expected; need to check for breaking internal readline

add readline to build, test to package

3b476d9

bump to version 0.3, mpi, pnetcdf, readline

c4e5807

build mpi, cuda and serial variants on Linux

4698ed7

fix glitch in build.sh that led to cpptraj itself not getting built

a92de0f

update location and sha to version 0.4

5c28c8e

minor tweaks to mkrelease

6814bbb

only use coreutils and nvcc_linux-64 on linux

d4344f8

seems to be able to compile on osx with openmp enabled

466deca

go back to using path: rather than url:/sha: for now

aa8cd72

hainm mentioned this pull request Jan 2, 2025

Create a conda package #1120

Open

dacase changed the base branch from master to conda January 2, 2025 16:37

hainm reviewed Jan 3, 2025

View reviewed changes

hainm mentioned this pull request Jan 3, 2025

Allow atommap rmsfit to proceed with partial atom maps #1114

Open

dacase mentioned this pull request Jan 4, 2025

CPPTRAJ should install headers into $CPPTRAJHOME/include #1121

Closed

mostly working, but need to get recursive headers into include/cpptra…

1a7f1a8

…j, for use by pytraj

no longer need to skip aarch64

14d4b0d

drroe mentioned this pull request Jan 14, 2025

Have configure and cmake install headers for libcpptraj #1129

Merged

Merge branch 'master' into conda

f7493ef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conda package for cpptraj #1125

Conda package for cpptraj #1125

dacase commented Jan 2, 2025

hainm commented Jan 3, 2025

dacase commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm Jan 3, 2025

hainm Jan 3, 2025

dacase Jan 4, 2025

hainm Jan 4, 2025

hainm Jan 4, 2025

hainm Jan 4, 2025

drroe commented Jan 3, 2025

dacase commented Jan 3, 2025

drroe commented Jan 4, 2025

drroe commented Jan 8, 2025

drroe commented Jan 8, 2025

drroe commented Jan 8, 2025

drroe commented Jan 14, 2025

dacase commented Jan 14, 2025

dacase commented Jan 15, 2025 •

edited

Loading



		rsync -a README.md config.h LICENSE dat bin lib test $PREFIX

		#!/bin/bash

		export CPPTRAJHOME=${CONDA_PREFIX}

Conda package for cpptraj #1125

Are you sure you want to change the base?

Conda package for cpptraj #1125

Conversation

dacase commented Jan 2, 2025

hainm commented Jan 3, 2025

dacase commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm commented Jan 3, 2025

hainm Jan 3, 2025

Choose a reason for hiding this comment

hainm Jan 3, 2025

Choose a reason for hiding this comment

dacase Jan 4, 2025

Choose a reason for hiding this comment

hainm Jan 4, 2025

Choose a reason for hiding this comment

hainm Jan 4, 2025

Choose a reason for hiding this comment

hainm Jan 4, 2025

Choose a reason for hiding this comment

drroe commented Jan 3, 2025

dacase commented Jan 3, 2025

drroe commented Jan 4, 2025

drroe commented Jan 8, 2025

drroe commented Jan 8, 2025

drroe commented Jan 8, 2025

drroe commented Jan 14, 2025

dacase commented Jan 14, 2025

dacase commented Jan 15, 2025 • edited Loading

dacase commented Jan 15, 2025 •

edited

Loading