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

Automatically choose all available Cuda compute targets > 50 #270

Merged
merged 4 commits into from
Jul 24, 2024

Conversation

atillack
Copy link
Member

@atillack atillack commented Jul 23, 2024

This PR adds the ability to compile for all available Cuda compute targets > 50.

Additionally, OVERLAP=ON and NUMWI=128 are now defaults - so in a well-configure Cuda environment all that should be needed now is make without having to specify other options.

I also fixed compile warnings related to strncat.

test_cuda.sh Outdated Show resolved Hide resolved
@rwxayheee
Copy link
Collaborator

rwxayheee commented Jul 24, 2024

Hi @atillack
Just a quick update on the capability of older cuda:

For cuda 11.0.3, now the compilation worked perfectly with gcc 9.1.0. Thanks so much for the change!

For cuda 10 and earlier, because only older gcc (below 8?) is compatible, filsystem.h is not in these gcc versions (or have a different name, I don't really know since which exact version of gcc, but this post might be able to provide the information). I was never able to compile AD-GPU with gcc below 8. (if you don't want users to compile with old cuda and gcc, we could provide some suggested combinations or the requirements in README, if that helps

filesystem.h was required by ./host/src/getparameters.cpp

@rwxayheee
Copy link
Collaborator

I was able to compile it with the following combinations:
(1) cuda 11.0.3, gcc 9.1.0
(2) cuda 11.1.1 to 11.8.0, gcc 10.3.0
(3) cuda 12.2, gcc 11.4.0

For testing, I did the basic docking with the provided files for ad4 and checked the results. I tested on the following hardware:
Tesla P100 V100 T4
(Ampere) A100

All look good to me. I will approve this PR if you think no further changes are to be made.

@atillack
Copy link
Member Author

@rwxayheee Thank you! Yeah, since without C++17 the filesystem operations we need would be a terribly unportable mess this is an easy choice 🙂

@atillack
Copy link
Member Author

I will update README.md separately to point out we now need at least Cuda 11 due to needing GCC >= 9.

@atillack
Copy link
Member Author

(this has technically been true for a while now since we switched to C++17)

Copy link
Collaborator

@rwxayheee rwxayheee left a comment

Choose a reason for hiding this comment

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

I was able to compile it with the following combinations:
(1) cuda 11.0.3, gcc 9.1.0
(2) cuda 11.1.1 to 11.8.0, gcc 10.3.0
(3) cuda 12.2, gcc 11.4.0

For testing, I did Vina basic docking with the provided files for ad4 and checked the results. I tested on the following hardware:
Tesla P100 V100 T4
(Ampere) A100

I approve the PR with the following notes for older cuda:
For cuda v10 and earlier, because only older gcc (below 8) is compatible, filsystem.h is not in these gcc versions (or have a different name. I was never able to compile AD-GPU with gcc below 8. filesystem.h was required by ./host/src/getparameters.cpp

@atillack
Copy link
Member Author

Thank you Amy!

@rwxayheee
Copy link
Collaborator

I will update README.md separately to point out we now need at least Cuda 11 due to needing GCC >= 9.

Sounds great, thank you! No hurry though, we can work on that tomorrow

@atillack
Copy link
Member Author

@rwxayheee I edited README.md in the develop branch (since those changes were there since commit 846dc2b).

@rwxayheee
Copy link
Collaborator

Hi @atillack
Thank you! I looked at f404aca. All look good to me.

@atillack atillack self-assigned this Jul 24, 2024
@atillack atillack requested review from diogomart and removed request for diogomart July 24, 2024 19:12
@atillack atillack merged commit 5aa1a3e into ccsb-scripps:develop Jul 24, 2024
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.

2 participants