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.

Sign up for GitHub

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

Jump to bottom

Support CUDA 12 #40

Merged
merged 18 commits into from
Apr 6, 2023
Merged

Support CUDA 12 #40

merged 18 commits into from
Apr 6, 2023

Conversation

conda-forge-admin
Copy link
Contributor

@conda-forge-admin conda-forge-admin commented Apr 4, 2023 •
edited by leofang
Loading

Hi! This is the friendly automated conda-forge-webservice.

I've rerendered the recipe as instructed in #39.

Here's a checklist to do before merging.

  • Bump the build number if needed.

Fixes #39 Close #36.

@conda-forge-webservices conda-forge-webservices bot mentioned this pull request Apr 4, 2023
Closed
@conda-forge-webservices
Copy link
Contributor

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.

@leofang leofang changed the title MNT: rerender WIP: Support CUDA 12 Apr 4, 2023
@leofang leofang marked this pull request as draft April 4, 2023 21:24
conda-forge-webservices[bot] and others added 2 commits April 4, 2023 21:24
@leofang
Copy link
Member

leofang commented Apr 4, 2023

@conda-forge-admin, please rerender

@leofang
Copy link
Member

leofang commented Apr 4, 2023

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits April 4, 2023 21:36
@leofang
Copy link
Member

leofang commented Apr 4, 2023

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 2 commits April 4, 2023 21:49
@leofang
Copy link
Member

leofang commented Apr 4, 2023

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2023

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/cutensor-feedstock/actions/runs/4612710773.

@leofang leofang changed the title WIP: Support CUDA 12 Support CUDA 12 Apr 5, 2023
@leofang leofang marked this pull request as ready for review April 5, 2023 01:16
@leofang
Copy link
Member

leofang commented Apr 5, 2023 •
edited
Loading

OK, this is ready to go for linux64/win64, but not for ppc64le/aarch64 yet because libcublas was not built on those platforms (conda-forge/libcublas-feedstock#5).

@leofang leofang mentioned this pull request Apr 5, 2023
Closed
bdice
bdice reviewed Apr 5, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

A few suggestions attached -- please treat my suggestions as non-blocking. Just thought I'd take a look at this PR since it relates to CUDA 12 work.

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml
- cudatoolkit {{ cuda_compiler_version }} # [cuda_compiler_version in ("10.2", "11.0")]
- cudatoolkit >=11.1,<12 # [cuda_compiler_version == "11.1"]
- {{ pin_compatible("cuda-version", max_pin="x") }}
- libcublas
run_constrained:
# Only GLIBC_2.17 or older symbols present
- __glibc >=2.17 # [linux]
Copy link
Contributor

@bdice bdice Apr 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is this constraint necessary? I think __glibc is a strong run-export of sysroot which should enforce this constraint (though I'm not totally sure if the cross-target logic in that recipe is enforcing that only on linux64?). A related run_exports came up in the CUDA 12 recipe for libcublas and we removed it: conda-forge/staged-recipes@1606772

Copy link
Member

Choose a reason for hiding this comment

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

It's a good question. This section has been there since ~3 yrs ago, and I bet we copied it from elsewhere. Things might be different now. Let me keep this thread open.

recipe/run_test.bat Show resolved Hide resolved
recipe/run_test.sh Show resolved Hide resolved
@leofang @bdice
Apply suggestions from code review
4a01f0b 
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@leofang leofang closed this Apr 6, 2023
@leofang leofang reopened this Apr 6, 2023
@leofang
Copy link
Member

leofang commented Apr 6, 2023

All green. Let's merge. Thanks Bradley & Alex!

@leofang leofang merged commit 3a74556 into conda-forge:main Apr 6, 2023
@leofang leofang mentioned this pull request Apr 6, 2023
Closed
@leofang
Copy link
Member

leofang commented Apr 6, 2023 •
edited
Loading

I regret the bad decision. As a tentative bandaid solution, I will mark the new packages as broken. We can unmark it after the infra catches up. Sorry for the noise.

@leofang leofang mentioned this pull request Apr 6, 2023
Merged
9 tasks
@leofang
Copy link
Member

leofang commented Apr 6, 2023

I regret the bad decision. As a tentative bandaid solution, I will mark the new packages as broken. We can unmark it after the infra catches up. Sorry for the noise.

I sent conda-forge/admin-requests#701.

@leofang leofang mentioned this pull request Apr 6, 2023
Closed
@leofang
Copy link
Member

leofang commented Apr 6, 2023 •
edited
Loading

What happened?

The problem here was that the cuda-version PRs (conda-forge/cuda-version-feedstock#3 and conda-forge/cudatoolkit-feedstock#89) from @jakirkham are not yet merged, and we haven’t patched old cudatoolkit to depend on cuda-version either. So, there's no mutual exclusion set up to distinguish CUDA 12 from the rest of the world, and when this new package was up users would get the CUDA 12 cutensor package from this PR alongside with an otherwise legal CUDA 11 environment (I checked that). I made the very same mistake that I just told everyone recently that we need to get the mutual exclusion done right on Day 1… I shouldn’t have merged after a long day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice left review comments

@adibbley adibbley adibbley left review comments

@JeremyWangNVDA JeremyWangNVDA Awaiting requested review from JeremyWangNVDA JeremyWangNVDA is a code owner

@jakirkham jakirkham Awaiting requested review from jakirkham jakirkham is a code owner

@leofang leofang Awaiting requested review from leofang leofang is a code owner

@mtjrider mtjrider Awaiting requested review from mtjrider mtjrider is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@conda-forge-admin, please rerender cuTENSOR: Support CUDA 12
4 participants
@conda-forge-admin @leofang @bdice @adibbley