conda-forge / petsc-feedstock

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

Update to use CUDA #193

Closed stefanozampini closed 4 months ago

stefanozampini commented 4 months ago

Checklist

Things to do:

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.

stefanozampini commented 4 months ago

Hypre complex failures (https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=921211&view=logs&j=517fe804-fa30-5dc2-1413-330699242c05&t=c7d0fcb4-3a10-5b26-dfea-e879a93fc2ab) should be fixed by https://gitlab.com/petsc/petsc/-/merge_requests/7497

stefanozampini commented 4 months ago

Should the tests be disabled when libcuda.so is not present? https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=921270&view=logs&jobId=696704cc-6fef-57a3-ea36-f27779b8cd5e&j=2ed0550f-e734-5ebf-52c6-1bfc19f5811c&t=9c969b19-a14a-5482-8fb9-78bdf745c3c6

dalcinl commented 4 months ago

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 commented 4 months ago

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 https://github.com/conda-forge/petsc-feedstock/pull/193#issuecomment-2077619300, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABHCKANY5PYFLOGYHOJ4DY3Y7EQ3BAVCNFSM6AAAAABGXHIA7SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXGYYTSMZQGA . You are receiving this because you authored the thread.Message ID: @.***>

stefanozampini commented 4 months ago

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 commented 4 months ago

. 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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

@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 :sweat_smile:.

dalcinl commented 4 months ago

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

leofang commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

@conda-forge-admin please rerender

github-actions[bot] commented 4 months ago

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

I considered the following status checks when analyzing this PR:

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