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

Bump to 3.1.0 #26

Merged
merged 12 commits into from
Nov 23, 2024
Merged

Bump to 3.1.0 #26

merged 12 commits into from
Nov 23, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Nov 20, 2024

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Managed to unvendor LLVM and make it build against LLVM 19 but it fails to run:

: CommandLine Error: Option 'print-pipeline-passes' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options

I've seen it before, I'll think about it more tomorrow. I still need to unvendor NVidia dependencies too.

Bump the recipe to triton 3.1.0, using the commit referenced
in PyTorch's .ci/docker/ci_commit_pins/triton.txt (since upstream
did not tag the release).  Backport a few patches to make it compatible
with LLVM/MLIR 19, as the 3.1.0 release was made against a mailine
commit between LLVM 18 and 19, and does not work with either of these
versions.

TODO: unvendor nvidia components
@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

For recipe/meta.yaml:

  • ℹ️ No valid build backend found for Python recipe for package triton using pip. Python recipes using pip need to explicitly specify a build backend in the host section. If your recipe has built with only pip in the host section in the past, you likely should add setuptools to the host section of your recipe.

@mgorny
Copy link
Contributor Author

mgorny commented Nov 21, 2024

Ok, so the problem with LLVM is that the build system somehow manages to simultaneously link to static and shared LLVM libraries, which results in duplicate symbols. My quick attempt to use just shared libraries fails, so instead I need to figure out where the shared libraries are coming from.

@conda-forge-admin
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/meta.yaml) and found it was in an excellent condition.

This package requires nvcc, so at least 12.0+, and it fails to compile
against 12.0.  Upstream uses 12.4.99 in this release.
@mgorny mgorny marked this pull request as ready for review November 21, 2024 18:13
@mgorny
Copy link
Contributor Author

mgorny commented Nov 21, 2024

A quick summary (roughly the same details as in comments and commit messages):

  • Unvendoring is done via environment variables read by setup.py. *_SYSPATH vars are used to locate stuff, CUDA-related vars don't seem to be used, beyond disabling downloads when they are defined.
  • I've had to backport a few upstream changes to make the package compatible with LLVM 19 release. Otherwise, it requires LLVM from a mainline commit between 18 and 19.
  • I've had to patch the build system not to link LLVM static libraries. It's linking static MLIR libraries which whose CMake exports have dependencies on LLVM shared library. As a result, both the static and shared libraries are used simultaneously, which leads to breakage.
  • I've had to switch to newer CUDA, as I've had build failures with 12.0. Upstram uses 12.4.99, I went straight for 12.6.
  • I've tested the resulting package against PyTorch 2.5.1, using this example: https://pytorch.org/tutorials/intermediate/torch_compile_tutorial.html. I've confirmed that torch.utils._triton.has_triton() evaluates to True (it did with PyPI-installed triton, it did not with the old version from conda-forge), and that nvcc is invoked.

@mgorny
Copy link
Contributor Author

mgorny commented Nov 21, 2024

CC @isuruf @hmaarrfk @danpetry

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

This looks very good already, thanks a lot. Some questions

recipe/meta.yaml Outdated Show resolved Hide resolved
recipe/meta.yaml Show resolved Hide resolved
recipe/meta.yaml Outdated Show resolved Hide resolved
Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Thank you very much!

@h-vetinari h-vetinari added the automerge Merge the PR when CI passes label Nov 23, 2024
@conda-forge-admin conda-forge-admin merged commit e4da8c3 into conda-forge:main Nov 23, 2024
7 checks passed
@conda-forge-admin
Copy link
Contributor

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!

@mgorny mgorny deleted the bump-3.1.0 branch November 23, 2024 06:17
@mgorny
Copy link
Contributor Author

mgorny commented Nov 23, 2024

Thanks!

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