-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
New package: cuda-histogram #27950
New package: cuda-histogram #27950
Conversation
0b9aa92
to
014bccb
Compare
Hi! This is the staged-recipes linter and I found some lint. File-specific lints and/or hints:
|
Hi! This is the friendly automated conda-forge-linting service. I failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11443235746. |
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 ( |
Hi! This is the staged-recipes linter and your PR looks excellent! 🚀 |
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 ( Here's what I've got... For recipes/cuda-histogram/meta.yaml:
|
e0be96d
to
bb96976
Compare
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 ( |
number: 0 | ||
skip: True # [win or osx or py<38] | ||
|
||
requirements: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth giving https://conda-forge.org/docs/maintainer/knowledge_base/#cuda-builds a read if you haven't already. I don't have any great advice here, but it might also be worth looking at how other tools that enable CUDA support have their conda-forge builds configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how to properly build CUDA enabled packages on conda-forge yet, but the failure on
+ pip check
cuda-histogram 0.1.0 requires cupy-cuda12x, which is not installed.
is coming from the PyPI package dependency on cupy-cuda12x
dependencies = [
"cupy-cuda12x>=13.1.0",
...
]
Nothing provides this, but I don't know how to get around the dependency check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also cc @jakirkham given that you added cupy
to conda-forge in the first place (huge thanks there! 🙇) in PR #9959.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also forgot to mention, we have some more up-to-date docs
We have an issue open to refresh the webpage docs ( conda-forge/conda-forge.github.io#1927 ), which we are a bit behind on. Sorry about that 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah and one more suggestion here
Regarding the CI failure. This is coming from pip check
. Basically pip
is saying cupy-cuda12x
is not installed, which is true as we just install it as cupy
. In conda, packages depend on CUDA so have this information encoded as a dependency of the package. Currently wheels lack a way to do this; so, authors have resorted to encoding this information in the wheel name (like cupy-cuda11x
and cupy-cuda12x
)
To fix pip check
, would recommend adding a patch in the recipe to change pyproject.toml
to use the canonical name like so. That would fix the error
dependencies = [
- "cupy-cuda12x>=13.1.0",
+ "cupy>=13.1.0",
"awkward>=2.6.3",
"numpy>=1.22.0",
"scipy>=1.1.0",
"hist>=2",
"boost-histogram",
]
recipes/cuda-histogram/meta.yaml
Outdated
- python | ||
- awkward >=2.6.3 | ||
- cupy >=13.1.0 | ||
- cuda-nvcc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the user need the CUDA compiler at runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume strictly no, but I'll let @Saransh-cpp comment on that.
recipes/cuda-histogram/meta.yaml
Outdated
build: | ||
script: {{ PYTHON }} -m pip install . -vv --no-deps --no-build-isolation | ||
number: 0 | ||
skip: True # [win or osx or py<38] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW conda-forge is on Python 3.9+. So we can probably simplify this like so
skip: True # [win or osx or py<38] | |
skip: True # [win or osx] |
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 ( For recipes/cuda-histogram/meta.yaml:
|
Sorry I am a bit inactive from work this week, but I will look into the comments soon. Thanks for all the help @matthewfeickert and @jakirkham, really appreciate it! |
The next |
Co-authored-by: Matthew Feickert <[email protected]>
021f3fd
to
397bfc4
Compare
@Saransh-cpp Seems that maybe the new metadata hadn't had time to propagate out everywhere when this was ran, but given that I can see it now $ pixi search mplhep_data
Using channels: conda-forge
mplhep_data pyhd8ed1ab_0
------------------------
Name mplhep_data
Version 0.0.4
Build pyhd8ed1ab_0
Size 5757500
License MIT AND OFL-1.1 AND LPPL-1.3c
Subdir noarch
File Name mplhep_data-0.0.4-pyhd8ed1ab_0.conda
URL https://conda.anaconda.org/conda-forge/noarch/mplhep_data-0.0.4-pyhd8ed1ab_0.conda
MD5 29e56ff931a12ce95d18614ad033a45c
SHA256 22648eeb812ce80bf2225054f9be80156bbdd567d780190383606a9ac2a421e2
Dependencies:
- python >=3.7 maybe try again in an hour? |
Thank you both! 🙏 Saransh caught a packaging bug with So we will want to sort that out to fix this issue |
Being fixed in conda-forge/mplhep_data-feedstock#5 |
That's merged now. 👍 So to give it time to propagate maybe try again in an hour (though you're in either the UK or India currently, @Saransh-cpp, so we can it till whenever you're awake and had some weekend rest to look at this again). :) |
Thank you both! 🙏 Was able to pick up the fixed package locally and confirm the fix. So have restarted CI |
@conda-forge/help-python, ready for review! |
Co-authored-by: jakirkham <[email protected]>
Thank you both! 🙏 |
The feedstock now lives here: https://github.com/conda-forge/cuda_histogram-feedstock Also @Saransh-cpp to join conda-forge and have permissions on this feedstock, please follow these steps: conda-forge/cuda_histogram-feedstock@34a2ff4#commitcomment-148713846 |
Thanks for all the help! |
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).