conda-forge / ambertools-feedstock

A conda-smithy repository for ambertools.
BSD 3-Clause "New" or "Revised" License
8 stars 14 forks source link

enable cuda build #144

Closed xiki-tempula closed 3 months ago

xiki-tempula commented 4 months ago

Checklist

conda-forge-webservices[bot] commented 4 months ago

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 (recipe) and found it was in an excellent condition.

xiki-tempula commented 4 months ago

@conda-forge-admin, please rerender

mattwthompson commented 4 months ago

119

xiki-tempula commented 4 months ago

The CUDA builds but the MPI+cuda failed due to No space left on device

xiki-tempula commented 4 months ago

@conda-forge-admin, please rerender

xiki-tempula commented 4 months ago

@mattwthompson This working fine when tested locally. Shall we merge this PR?

xiki-tempula commented 4 months ago

@mattwthompson I think we could potentially test cpptraj.cuda. However, I don't know how to test this only when there is cuda. I wonder if I can have some of your help? Thank you.

mattwthompson commented 4 months ago

I think it can be tucked away under a conditional like already happens for MPI: https://github.com/conda-forge/ambertools-feedstock/blob/bc93c747c80f8592d002ea0aa5ac321a82e9fe6f/recipe/run_test.sh#L50-L56

xiki-tempula commented 4 months ago

@conda-forge-admin, please rerender

xiki-tempula commented 4 months ago

@mattwthompson Ok, I have added a which and a lld test for cpptraj.cuda, which seems to pass.

xiki-tempula commented 4 months ago

@conda-forge-admin, please restart ci​

xiki-tempula commented 3 months ago

@conda-forge-admin, please rerender

xiki-tempula commented 3 months ago

@conda-forge-admin, please restart ci​

xiki-tempula commented 3 months ago

@mattwthompson Shall we merge this before working on 24? All the builds are passing and I have test for cpptraj.cuda.

mattwthompson commented 3 months ago

It's probably fine but I'd like somebody from the Amber community to chime in

xiki-tempula commented 3 months ago

@njzjz Do you mind give this PR a review? Thanks.

dacase commented 3 months ago

Is there some way I could provisionally do a conda install of this? It doesn't have to be before the PR is accepted.

Maybe I should just be building this myself locally(?) Are there instructions for doing that? Apologies for being so clueless -- I prepared conda (not conda-forge) packages years ago, but my grey cells are mostly gone in that (and other) areas.....

mattwthompson commented 3 months ago

You can re-configure the build process to store artifacts if they're not already there (I haven't checked), re-render, and then on the next build they're be available to download somewhere on the Azure pages that are linked in "CI" from this PR. Then you can install those as local conda packages

https://conda-forge.org/docs/maintainer/conda_forge_yml/#azure

Everything can be done locally (there's a build_locally.py file somewhere and/or information in the docs https://conda-forge.org/docs/maintainer/) but given the complexity of this feedstock I would worry there's a risk of false positives compared to what happens in automation

xiki-tempula commented 3 months ago

@dacase Building the linux version is quite easy, you just find a linux machine. clone the repo, run build_locally.py and select the relevant option. The linux build is run with docker so it is quite independent of your local environment. Then upload the .conda file to your local conda channel and you can then install it with conda. In our company, we use this branch to build amber, where I change the source to build from local tar.gz file. Then upload the .conda to company conda channel and deploy in our production environment where pmemd.cuda.mpi works as expected.

mattwthompson commented 3 months ago

@xiki-tempula could you merge the main branch into this once more? I don't want to accidentally revert #147

After that I'll merge tomorrow morning if nobody else got to it by then (or objects)