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

Update to use CUDA #193

Merged
merged 2 commits into from
May 2, 2024
Merged

Update to use CUDA #193

merged 2 commits into from
May 2, 2024

Conversation

stefanozampini
Copy link
Contributor

@stefanozampini stefanozampini commented Apr 24, 2024

Checklist

Things to do:

  • how to enable CUDA 12?

@conda-forge-webservices
Copy link

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.

@dalcinl
Copy link
Contributor

dalcinl commented Apr 25, 2024

Should the tests be disabled when libcuda.so is not present?

Maybe there is a way to link with libcuda.so from the stubs, perhaps via setting LIBRARY_PATH (for the GCC+linker) and LD_LIBRARY_PATH (for the dynamic linker) ? Or there are not even stubs to search for?

@stefanozampini
Copy link
Contributor Author

stefanozampini commented Apr 25, 2024 via email

@stefanozampini
Copy link
Contributor Author

stefanozampini commented Apr 25, 2024

Maybe add __cuda as the set of requirements for the test?

On Thu, Apr 25, 2024, 18:50 Lisandro Dalcin @.> wrote: Should the tests be disabled when libcuda.so is not present? Maybe there is a way to link with libcuda.so from the stubs, perhaps via setting LIBRARY_PATH (for the GCC+linker) and LD_LIBRARY_PATH (for the dynamic linker) ? Or there are not even stubs to search for? — Reply to this email directly, view it on GitHub <#193 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHCKANY5PYFLOGYHOJ4DY3Y7EQ3BAVCNFSM6AAAAABGXHIA7SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXGYYTSMZQGA . You are receiving this because you authored the thread.Message ID: @.>

Nope, it didn't work. So maybe we will disable the test for those archs?

Also, by looking at the logs, PETSc uses rpath for /usr/local/cuda/lib64 https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=922006&view=logs&j=2ed0550f-e734-5ebf-52c6-1bfc19f5811c&t=9c969b19-a14a-5482-8fb9-78bdf745c3c6&l=4141. I think we should use runpath, or try to use a similar trick as for -L$PREFIX/lib for the other dependencies. @dalcinl What do you think?

@dalcinl
Copy link
Contributor

dalcinl commented Apr 25, 2024

. I think we should use runpath

I'm not sure what do you mean exactly. Are you talking about passing -Wl,--enable-new-dtags to use new newer RUNPATH semantics instead of the older RPATH semantics in shared libraries? In general, I would agree, although with the particular of CUDA, I'm not fully aware of all the gory details.

@stefanozampini
Copy link
Contributor Author

@dalcinl Builds are clean, I'm skipping running the compiled tests for aarch64 for now. We can figure out the cross-compilation issue with curand later

@dalcinl
Copy link
Contributor

dalcinl commented Apr 29, 2024

Stefano, if you know for a fact a build is broken and will not work, I believe the usual conda-forge approach to it is to skip it, simply because broken package files should not be published. Or this is just a CI testing issue, and the packages to work if installed in systems with newer enough GLIBC? Have you tried? In that case, then yes, we can move forward. Please confirm (or we can discuss it in person in a couple hours).

@dalcinl
Copy link
Contributor

dalcinl commented Apr 29, 2024

@stefanozampini Just in case, can do it with two commits? One for the recipe/config changes, and another for the re-rendering changes from conda-smithy? Maybe this is not really necessary, but that's how I've always done it, and I prefer to follow established patterns in things I do not understand in full 😅.

@dalcinl
Copy link
Contributor

dalcinl commented Apr 29, 2024

@leofang Any chance you can give a quick glance to the changes to meta.yml here?

@leofang
Copy link
Member

leofang commented Apr 29, 2024

You have CUDA 12 support added to your recipe, but no CI/CD pipelines spawned. I suggest that you manually add the CUDA 12 migrator to .ci_support/migrations and rerender.

For complex recipes like this one, cupy-feedstock can be a good reference (for supporting both CUDA 11/12 which have different file layouts + many CUDA packages + Python + ...), but no big issue from a quick glance other than the missing CIs.

@dalcinl
Copy link
Contributor

dalcinl commented Apr 29, 2024

For complex recipes like this one, cupy-feedstock can be a good reference

We did, but we were not sure about the CUDA 12 migration, so @stefanozampini leaved it out. Now we know better.
Many thanks, @leofang !

@leofang
Copy link
Member

leofang commented Apr 29, 2024

No worries, last time I heard we're preparing a doc refresher for conda-forge CUDA packages. It might take some time to finalize, sorry for the learning curve. Before it lands, feel free to ping me or anyone from @conda-forge/cuda to ask for help.

@stefanozampini
Copy link
Contributor Author

stefanozampini commented Apr 29, 2024

@leofang, I see cupy feedstock only builds with 11.8 or 12.4 https://github.com/conda-forge/cupy-feedstock/blob/main/recipe/meta.yaml#L37. Is the idea to use 11.8 or 12.4 only?

@leofang
Copy link
Member

leofang commented Apr 29, 2024

CuPy was built with both CUDA 11.8 & 12.4. The built packages can serve both CUDA 11.x and 12.x users, not limited to the build versions (but obviously some selected features will require a newer CUDA version).

@stefanozampini
Copy link
Contributor Author

thanks. So, to support the same in PETSc, I need to copy-paste the ignore_run_exports paraphernalia and related run_constrained stuff that are in cupy?

@stefanozampini
Copy link
Contributor Author

@conda-forge-admin please rerender

@dalcinl dalcinl added the automerge Merge the PR when CI passes label May 2, 2024
@github-actions github-actions bot merged commit 7a7f56f into conda-forge:main May 2, 2024
46 checks passed
Copy link
Contributor

github-actions bot commented May 2, 2024

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants