Skip to content

enable cuda build#144

Merged
mattwthompson merged 3 commits into
conda-forge:mainfrom
xiki-tempula:patch-1
Jun 14, 2024
Merged

enable cuda build#144
mattwthompson merged 3 commits into
conda-forge:mainfrom
xiki-tempula:patch-1

Conversation

@xiki-tempula

@xiki-tempula xiki-tempula commented May 30, 2024

Copy link
Copy Markdown
Contributor

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@mattwthompson

Copy link
Copy Markdown
Member

#119

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

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

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@xiki-tempula xiki-tempula marked this pull request as ready for review May 31, 2024 10:50
@xiki-tempula xiki-tempula changed the title enable cude build enable cuda build May 31, 2024
@xiki-tempula

Copy link
Copy Markdown
Contributor Author

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

@mattwthompson mattwthompson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The green checks are good and the changes pass the eye test, but I'd like one more person to either skim the changes or test them locally.

Are there any executables that can be turned on in this file with the inclusion of CUDA?

https://github.com/conda-forge/ambertools-feedstock/blob/main/recipe/run_test.sh

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@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

Copy link
Copy Markdown
Member

I think it can be tucked away under a conditional like already happens for MPI:

if [[ $mpi == nompi ]]; then
echo "no MPI, skipping sander.MPI check"
elif [[ $mpi == openmpi ]]; then
echo "OpenMPI cannot work correctly; see https://github.com/conda-forge/openmpi-feedstock/issues/152"
else
sander.MPI --version
fi

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

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

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please restart ci​

xiki-tempula and others added 2 commits June 13, 2024 15:51
MNT: Re-rendered with conda-build 24.5.1, conda-smithy 3.36.1, and conda-forge-pinning 2024.05.29.16.17.59

Update build.sh
Update meta.yaml
Update meta.yaml
Update conda-forge.yml
MNT: Re-rendered with conda-build 24.5.1, conda-smithy 3.36.1, and conda-forge-pinning 2024.05.30.18.15.52

test cpptraj.cuda
update test
fix ldd
@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please rerender

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

@conda-forge-admin, please restart ci​

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

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

@mattwthompson

Copy link
Copy Markdown
Member

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

@xiki-tempula

Copy link
Copy Markdown
Contributor Author

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

@dacase

dacase commented Jun 13, 2024

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Member

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

xiki-tempula commented Jun 13, 2024

Copy link
Copy Markdown
Contributor Author

@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

Copy link
Copy Markdown
Member

@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)

numpy:
- '1.22'
openmpi:
- '5'

@xiki-tempula xiki-tempula Jun 13, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattwthompson I have already merged the main to this branch and rerendered the branch. You could see that openmpi is version 5 and zlib is 1

@mattwthompson mattwthompson merged commit 725d666 into conda-forge:main Jun 14, 2024
@xiki-tempula xiki-tempula deleted the patch-1 branch June 15, 2024 10:27
@mikemhenry mikemhenry mentioned this pull request Feb 12, 2025
5 tasks
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

Successfully merging this pull request may close these issues.

4 participants