[BUG] Applying patch cupy 3218 to build to get full CUDA 10.2 support.

conda-forge / cupy-feedstock

A conda-smithy repository for cupy.

BSD 3-Clause "New" or "Revised" License

5 stars 23 forks source link

[BUG] Applying patch cupy 3218 to build to get full CUDA 10.2 support. #41

Closed rlratzel closed 4 years ago

rlratzel commented 4 years ago

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

Fixes error exclusive to CUDA 10.2 environments where cupy raises an error:

catastrophic error: cannot open source file "cuda_fp16.h"

This PR cherry-picks commits to add full CUDA 10.2 support, which was added in cupy commit https://github.com/cupy/cupy/commit/d654524fa1c5aae4df39090d148f5eadce4ebb2c

The cupy package was built locally and installed in a CUDA 10.2 environment, where the previously-failing test then passed.

conda-forge-linter commented 4 years 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.

leofang commented 4 years ago

@jakirkham Another thing that has been haunting us for quite a while: Do we want to do our own patches in the absence of CUDA-enabled CIs? This always makes me nervous...I prefer (as always) the burden to be on the PFN-CuPy team, who has the resource and bandwidth to do fixes properly and permanently, not on conda-forge maintainers...

This is not my hard "NO" though, just wondering what you think and what other popular recipes do.

leofang commented 4 years ago

Also, please follow John's comment on the patching. I'd suggest you to apply the patch from cupy/cupy#2910 directly, instead of doing a git cherry-pick. It's nerve wracking to me as it'd be hard to maintain and keep track...

See how we did a patch for CUB earlier: #32.

jakirkham commented 4 years ago

The commits actually don't appear to apply cleanly unfortunately. Since it sounds like there is a new CuPy release that is imminent, maybe we should just to try get a patch in there. I've taken a pass at doing this in PR ( https://github.com/cupy/cupy/pull/3218 ). @rlratzel @leofang, could you please check that my patch looks ok? 🙂

jakirkham commented 4 years ago

Could you please wait for a little while and target v7.3.0, which just gets released a few hours ago? The bot will generate a PR, and you can rebase on top of that.

Looks like v7.3.0 was already cut ( https://github.com/cupy/cupy/releases/tag/v7.3.0 ).

Edit: Opened issue ( https://github.com/regro/cf-scripts/issues/896 ) to figure out why the bot hasn't sent a PR.

jakirkham commented 4 years ago

Leo, for context @rlratzel is a colleague of mine. We ran into a CUDA 10.2 issue in CuPy that was impacting cuML. So he was trying to help out here in applying that fix.

conda-forge-linter commented 4 years ago

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

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-linter commented 4 years 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.

leofang commented 4 years ago

@conda-forge-admin, please rerender

github-actions[bot] commented 4 years ago

Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do.

leofang commented 4 years ago

We ran into a CUDA 10.2 issue in CuPy that was impacting cuML. So he was trying to help out here in applying that fix.

Thanks for filling me in, John! Sorry to both of you if my reaction was perceived unfriendly to you. I didn't mean that...

leofang commented 4 years ago

Once the CI passes, I'll merge and then update the recipe to v7.3.0.

rlratzel commented 4 years ago

Sorry to both of you if my reaction was perceived unfriendly to you. I didn't mean that...

No worries at all, thanks for your guidance.

leofang commented 4 years ago

All CI passes. Thanks again for help, Rick! Merging...

jakirkham commented 4 years ago

Thanks Leo! 😀

jakirkham commented 4 years ago

Sorry to both of you if my reaction was perceived unfriendly to you. I didn't mean that...

Not at all. The points you raised seemed valid.

Still mulling over how we can improve testing around CuPy changes.