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

cupy v13.0.0 #241

Merged
merged 38 commits into from
Jan 23, 2024
Merged

cupy v13.0.0 #241

merged 38 commits into from
Jan 23, 2024

Conversation

regro-cf-autotick-bot
Copy link
Contributor

@regro-cf-autotick-bot regro-cf-autotick-bot commented Jan 18, 2024

Close #229.

It is very likely that the current package version for this feedstock is out of date.

Checklist before merging this PR:

  • Dependencies have been updated if changed: see upstream
  • Tests have passed
  • Updated license if changed and license_file is packaged

Information about this PR:

  1. Feel free to push to the bot's branch to update this PR if needed.
  2. The bot will almost always only open one PR per version.
  3. The bot will stop issuing PRs if more than 3 version bump PRs generated by the bot are open. If you don't want to package a particular version please close the PR.
  4. If you want these PRs to be merged automatically, make an issue with @conda-forge-admin,please add bot automerge in the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.
  5. If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

Pending Dependency Version Updates

Here is a list of all the pending dependency version updates for this repo. Please double check all dependencies before merging.

Name Upstream Version Current Version
cudatoolkit 12.0.0 Anaconda-Server Badge

Dependency Analysis

We couldn't run dependency analysis due to an internal error in the bot, depfinder, or grayskull. :/ Help is very welcome!

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/7573336059, please use this URL for debugging.

@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 marked this pull request as draft January 18, 2024 18:06
@leofang
Copy link
Member

leofang commented Jan 18, 2024

To address #229, I wanna raise a question to @conda-forge/cupy: With CuPy v13 we can move all run dependencies to run_constrained because all CUDA libs are now optional. While this would help deployment on edge devices etc, I think it's a breaking change in terms of installation UX, as users need to explicitly install all dependencies manually.

My proposal:

  • Add a new output cupy-core which has everything run-constrained.
    • FYI, XXXXX-core is a common naming scheme for packaging core/essential capabilities of an otherwise extended project on conda-forge
  • Keep cupy as before (with dependencies listed in run), to avoid breaking change
  • Document cupy-core: Update conda installation guide cupy/cupy#8129

Thoughts?

@conda-forge-webservices

This comment was marked as resolved.

@conda-forge-webservices

This comment was marked as outdated.

@leofang

This comment was marked as outdated.

@leofang

This comment was marked as outdated.

conda-forge-webservices[bot] and others added 2 commits January 19, 2024 04:32
@leofang

This comment was marked as outdated.

This comment was marked as outdated.

@bdice
Copy link
Contributor

bdice commented Jan 19, 2024

@leofang I like the cupy-core proposal. I agree the installation UX of including CUDA package dependencies by default is worth preserving.

@vyasr
Copy link

vyasr commented Jan 19, 2024

I concur with the benefits of having two separate packages. I think cupy-core would be especially helpful to other libraries that use a subset of cupy's functionality internally since they could specify the subset of required dependencies, while preserving the benefits of having all dependencies installed for cupy end-users.

@leofang
Copy link
Member

leofang commented Jan 19, 2024

@conda-forge-admin, please rerender

Copy link
Contributor

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/cupy-feedstock/actions/runs/7585579292.

@leofang
Copy link
Member

leofang commented Jan 19, 2024

@conda-forge-admin, please rerender

@leofang
Copy link
Member

leofang commented Jan 19, 2024

@beckermr @jakirkham I am unable to disable Python 3.8 which CuPy v13 no longer supports. Could this be a bug in conda-smithy? Could this relate to the Python 3.12 migration issue that we were unable to fix (#234)?

@jakirkham
Copy link
Member

@conda-forge-admin , please re-render

@jakirkham
Copy link
Member

Add a dummy top-level requirements section with build and host.

Did essentially this and added python, which seems to do the trick

recipe/meta.yaml Outdated Show resolved Hide resolved
Comment on lines 37 to -137
requirements:
build:
- {{ compiler("c") }}
- {{ compiler("cxx") }}
- {{ compiler("cuda") }}
#- cuda-nvcc
- sysroot_{{ target_platform }} 2.17 # [linux]
- cross-python_{{ target_platform }} # [build_platform != target_platform]
- python # [build_platform != target_platform]
- cython >=0.29.22,<3 # [build_platform != target_platform]
# TODO: clean up
{% if cuda_major >= 12 %}
- cuda-driver-dev # [linux and build_platform != target_platform]
- cuda-cudart-dev # [build_platform != target_platform]
- cuda-nvrtc-dev # [build_platform != target_platform]
- cuda-nvtx-dev # [build_platform != target_platform]
- cuda-profiler-api # [build_platform != target_platform]
- cuda-cccl_{{ build_platform }} # [build_platform != target_platform]
- libcublas-dev # [build_platform != target_platform]
- libcufft-dev # [build_platform != target_platform]
- libcurand-dev # [build_platform != target_platform]
- libcusolver-dev # [build_platform != target_platform]
- libcusparse-dev # [build_platform != target_platform]
{% endif %}
# optional dependencies for CUDA 11.2+
- cudnn >=8.0.*,<9 # [build_platform != target_platform and (not ((aarch64 or ppc64le) and (cuda_compiler_version or "").startswith("12")))]
- nccl >=2.8,<3 # [build_platform != target_platform]
- cutensor >=1.4,<2 # [build_platform != target_platform]
- cusparselt ~=0.2.0.0 # [build_platform != target_platform and (linux64 or aarch64 or win) and (cuda_compiler_version or "").startswith("11")]

host:
- python
- pip
- setuptools
- cython >=0.29.22,<3
- fastrlock >=0.5
- cuda-version {{ cuda_compiler_version }}
- nvtx-c # [win64]
# TODO: clean up
{% if cuda_major >= 12 %}
- cuda-driver-dev # [linux]
- cuda-cudart-dev
- cuda-nvrtc-dev
- cuda-nvtx-dev
- cuda-profiler-api
- cuda-cccl_{{ target_platform }}
- libcublas-dev
- libcufft-dev
- libcurand-dev
- libcusolver-dev
- libcusparse-dev
{% endif %}
# optional dependencies
# TODO: see https://github.com/conda-forge/cudnn-feedstock/issues/58
- cudnn >=8.0.*,<9 # [not ((aarch64 or ppc64le) and (cuda_compiler_version or "").startswith("12"))]
- nccl >=2.8,<3 # [linux]
- cutensor >=1.4,<2
- cusparselt ~=0.2.0.0 # [(linux64 or aarch64 or win) and (cuda_compiler_version or "").startswith("11")]
Copy link
Member

Choose a reason for hiding this comment

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

Think we may want to keep these top-level requirements at least for build & host

Copy link
Member

Choose a reason for hiding this comment

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

Why? TBH I don't quite understand the purpose of top-level requirements, when there are multiple outputs. Seems like just hacks to me for bowing to the limitations of conda-build & co? But what do we wish to achieve here, when it's working fine now?

Copy link
Member

Choose a reason for hiding this comment

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

The original idea of the multi-outputs recipe was to largely handle everything (like the build, dependencies at build time, etc.) at the top-level

The outputs sections themselves were mainly geared around taking the resulting contents of $PREFIX and slicing them up in different ways to create several (potentially interrelated) packages

Over time people have added more things to the subpackages sections (different requirement stages, more complicated build & install patterns, etc.). As we have seen these later additions are incomplete

This is why we see issues with Python and PIP_* environment variables, misunderstandings by the build tooling about how to build the recipe, etc.

Hewing close to the original model will minimize the chance of niche issues cropping up

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, John, for sharing the background. My preference is to keep the recipe as-is and see what else issue we could run into, unless you have strong opinion otherwise. The status quo is cleaner and makes better sense to reflect the intention IMHO. I am also very curious to see how this would involve into, once the new recipe format is up and running.

@leofang leofang marked this pull request as ready for review January 22, 2024 17:51
@leofang
Copy link
Member

leofang commented Jan 22, 2024

The PR is ready. Will merge later tonight / tomorrow unless there are objections.

Lessons learned in this PR

  • Cannot skip PY38:
    • solution: Add python as a host requirement, as well as the skip condition, to the top-level and all relevant sub packages
    • xref: cupy v13.0.0 #241 (comment)
  • cutensor 1.* seems to be cached somewhere
  • Jinja variable {{ PYTHON }} does not resolve in a multi-output recipe
    • solution: Hard-code the host Python path manually
  • Included patch gets outdated (cc: @kmaehashi for vis)
  • CUDA source compilation failed on Windows:
    • solution: Increase the swap size via adding SET_PAGEFILE to conda-forge.yaml.

@leofang
Copy link
Member

leofang commented Jan 23, 2024

Merging, thanks @beckermr @jaimergp @jakirkham for help and @jakirkham @vyasr @bdice for reviewing the cupy-core proposal. In we go!

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.

Support lazy import in v13
7 participants