Skip to content
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 a conda package #1120

Open
drroe opened this issue Dec 19, 2024 · 17 comments
Open

Create a conda package #1120

drroe opened this issue Dec 19, 2024 · 17 comments
Assignees

Comments

@drroe
Copy link
Contributor

drroe commented Dec 19, 2024

This would make it easier for cpptraj to be part of Amber Classic

@hainm
Copy link
Contributor

hainm commented Dec 19, 2024

@drroe please include libcpptraj.so and header files so pytraj can use too. Currently pytraj doesn't have its own conda package anymore (only in AmberTools).

@drroe
Copy link
Contributor Author

drroe commented Dec 19, 2024

Great idea @hainm thanks!

@drroe drroe self-assigned this Dec 19, 2024
@dacase
Copy link

dacase commented Dec 19, 2024

@hainm Where are the header files pytraj needs? Does cpptraj put them in $CPPTRAJHOME/include?

@drroe
Copy link
Contributor Author

drroe commented Dec 19, 2024

Where are the header files pytraj needs? Does cpptraj put them in $CPPTRAJHOME/include?

It should, but it doesn't. I'll have to modify the install process to be something more sane. I think right now pytraj and anything that uses libcpptraj.so looks in the source directory (which is not great if you've installed outside the source directory).

@hainm
Copy link
Contributor

hainm commented Dec 19, 2024

@hainm Where are the header files pytraj needs? Does cpptraj put them in $CPPTRAJHOME/include?

hi @dacase it has been a while that we have not chatted (I am very sorry).
Currently pytraj looks for header files either in $CPPTRAJ_HEADERDIR or $CPPTRAJHOME/src
https://github.com/Amber-MD/pytraj/blob/d38bafaf302835d3f9748db4e796b5e1663420d9/scripts/base_setup.py#L290-L325

(and it looks for libcpptraj.so either in $CPPTRAJ_LIBDIR or $CPPTRAJHOME/lib folder). Cheers.

@hainm
Copy link
Contributor

hainm commented Dec 19, 2024

If pytraj is in AmberTools, it looks for

CPPTRAJ_LIBDIR = os.path.join(AMBERHOME, 'lib')
CPPTRAJ_HEADERDIR = os.path.join(AMBERHOME, 'AmberTools', 'src',
                                             'cpptraj', 'src')

@dacase
Copy link

dacase commented Dec 19, 2024

OK: it turns out to be pretty simple to build a conda package, at least for cpptraj itself -- i.e. not yet with MPI or cuda support. I had to tweak the readline code a bit, but that is all. I've not yet tested the result, but that comes next.

I am deliberately not giving any details now, because I recognize that this is not a good time for Dan to have anything else on his plate. After the holidays are over, I'll post updated info here.

@dacase
Copy link

dacase commented Dec 21, 2024

Update: I now have a cpptraj conda package built for osx-64 as well as linux-64. Only changes were another small tweak to readline, plus one missing header.

On linux, tests all pass, but 5 of them are skipped....I need to try to figure out if this is expected, or if something else needs fixing.

I'm hoping that Tom can come through with his promise to get me access to a mac with Apple Silicon -- that is a pretty popular platform, but I don't have one myself.

@dacase
Copy link

dacase commented Dec 23, 2024

Various notes about the current status of a cpptraj conda package; this is mostly notes to myself and Dan.

  • I pretty much have everything working (but see below) on linux-64: configured with -readline -openmm -openblas -fftw3 -zlib -bzlib -shared -arpack. Separate executables are created by adding -mpi or -cuda.
  • The readline support comes from the conda-forge readline package, which uses nucurses rather than termcap. I rarely use the readline capability of cpptraj myself, and don't know how to test this.
  • The basic cpptraj executable seems to pass (or skip) all the tests. I'm unsure how to test cpptraj.MPI or cpptraj.cuda (although I see that there are Makefile targets that should do this.) @drroe and others will have to put this package through a workout on various hardware, including environments with older and newer GPU cards.
  • I'm also unsure what to do with cpptraj.OMP, which is not built as part of the AmberTools distribution, at least by default. (See How best to handle cpptraj vs. cpptraj.{MPI,OMP,cuda} #1123 for a separate issue on this subject.)
  • Right now, I am using gcc 11.4, since NVCC doesn't support higher versions; I'm using CUDA SDK version 11.8, since conda doesn't seem to support higher versions. It's probably possible to (say) build cpptraj itself with gcc14, and use gcc11 for cpptraj.cuda, but I haven't yet figured out the syntax for doing that.
  • The changes are in the conda branch at github.com/Amber-MD/cpptraj. There are only two one-line changes to the cpptraj code: adding a missing header in Matrix.h, and changing the location <readline.h> to <readline/readline.h> in Readline.cpp. Both changes look correct to me, but we will need to see if they break anything in non-conda builds before merging into master.
  • The current versions of the conda packages are available at https://anaconda.org/dacase/cpptraj. These are only there for developer testing, and may disappear or get changed at any time.
  • At some point, the existing recipe should be converted to a conda-forge one. I think I understand the basics of doing this, but help from those more experienced would be a big help.

@dacase
Copy link

dacase commented Dec 26, 2024

Some more notes:

  • I now have openmp working on osx-64, i.e. making cpptraj.OMP. This only works for conda builds. If a user wants to compile cpptraj.OMP outside the conda universe, it looks like addtional libraries need to be available, which depend on the version of clang++ being used.
  • It looks like building cpptraj.cuda on OSX would require access to a Mac with an NVIDIA card installed. Does anyone have such a beast? Or have there been feature requests for this capability?
  • I've been using the Makefile to build the conda packages (see recipe/build.sh in the conda branch). Is there an argument to switch to cmake? (D'oh: I can partially answer my own question -- the make and cmake builds seem to do the same thing. So it's really a matter of whether @drroe wants to continue to maintain both.)

@drroe
Copy link
Contributor Author

drroe commented Jan 2, 2025

The changes are in the conda branch at github.com/Amber-MD/cpptraj. There are only two one-line changes to the cpptraj code: adding a missing header in Matrix.h, and changing the location <readline.h> to <readline/readline.h> in Readline.cpp. Both changes look correct to me, but we will need to see if they break anything in non-conda builds before merging into master.

I don't see this branch on https://github.com/Amber-MD/cpptraj/branches/all - is it in a fork somewhere? Once I have access to it I can try to test if there are any issues with "regular" builds. The readline change may just require me modifying the include compiler flag for the built-in readline build; I can test this once I have access to the code.

@dacase
Copy link

dacase commented Jan 2, 2025

I need to learn more about pull requests....I created one, but it ended up saying that it was a request to merge the conda branch at gihub.com/dacase/cpptraj to the master branch at github.com/Amber-MD/cpptraj. I assume that is not what either one of us wants, but I don't yet see how to ask the PR to create a new branch at Amber-MD.

An alternative is for you to visit github.com/dacase to see what is there.

@hainm
Copy link
Contributor

hainm commented Jan 2, 2025

I assume that is not what either one of us wants, but I don't yet see how to ask the PR to create a new branch at Amber-MD.

image

@dacase You could always create a new branch by typing the name here. By the way, I've created the "conda" branch for you and you could update the base branch in this PR: #1125

@dacase
Copy link

dacase commented Jan 2, 2025

OK: but what if I was making a pull request to a target repository where I had no write access?

And, in spite of formal permissions, I view Amber-MD/cpptraj as Dan's bailiwick, and I don't generally want to make changes without his permission.

I did update the target branch in PR #1125.

@drroe
Copy link
Contributor Author

drroe commented Jan 2, 2025

So my development process is I have my own fork of Amber-MD/cpptraj: https://github.com/drroe/cpptraj

I make my branches from that repository, e.g. https://github.com/drroe/cpptraj/tree/prepareforleap.remote.buildatom

When I'm ready, I can make a merge request for my local repository branch (i.e. drroe/cpptraj) to the main repository (Amber-MD/cpptraj). If you're on the branch GitHub page, there should be a green button near the top that says 'Contribute' which let's you open a pull request.

@dacase
Copy link

dacase commented Jan 2, 2025

Thanks. I used the "create pull request" button (under pull requests), rather than the "contribute button (which is not green in my browser). But the latter seems more correct.

It looks like PR #1125 (as now modified) should do what you want. This PR has a bunch of new, conda-related files, plus two files that make minor diffs to files in the current master branch. You should see if those changes look OK, and don't break any of your current builds.

Further steps, as time allows:

  • create a new conda environment, and conda install dacase::cpptraj. Try out the tests, see if readline is working interactively as you expect it to, etc. @hainm could see if library and other files that pytraj needs are OK. My expectation is that a number of (hopefully small) tweaks are still needed.
  • If/when we are happy with things, conversion to a conda-forge package would be in order. This would give us access to platforms beyond linux-64 and osx-64, and would ensure that we are meeting standards expected of conda-forge packages. I've looked at the existing AmberTools and ParmEd feedstocks, and I think I know how to proceed. But if we could get @swails or others with more experience to help, that would be great.

@dacase
Copy link

dacase commented Jan 10, 2025

Some progress: I have my shiny new MacBook Air with an M2 chip, and can now start making conda packages for osx-arm64. All is not yet perfect: I got osx-64 to compile with -openmp, but no comparable luck yet with osx-arm64. But I have error messages about missing softlinks, so I am hoping I can track these down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants