New cuda #157
Merged
New cuda #157
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wolfv
requested review from
FarhanTejani,
ghego,
gilbertfrancois,
h-vetinari,
hajapy,
hmaarrfk,
jschueller,
njzjz,
waitingkuo and
xhochy
as code owners
|
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 ( |
Member
Author
|
@hmaarrfk sorry for dropping your commits, I just wanted to see what the CI says. I will try to make a proper history once this is passing (if it ever does! :) |
Member
Author
|
Argh, fails locally with |
Contributor
|
i definitely don't care about my commits. I'm more than happy to see a clean history. |
Contributor
|
Where is CUDA_HOME defined? I think ti should be set to |
Member
Author
|
I find all the CUDA stuff in |
hmaarrfk
reviewed
Oct 27, 2021
Contributor
|
The meta file might need a bit of refreshing. Do look through the deleted lines. Many of them are from a recent refresh when I figured i was hitting build issues. You will also need the patch I recently added. |
Member
Author
|
thanks for the heads up! I will re-add all the red lines, for sure. Or feel free to push to this PR if you want. |
Contributor
|
seems to be working |
|
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 recipe:
|
Member
Author
|
indeed, I discovered the need for |
Member
Author
wolfv
commented
Oct 28, 2021
wolfv
commented
Oct 28, 2021
wolfv
commented
Oct 28, 2021
wolfv
commented
Oct 28, 2021
Member
Author
|
Also I am wondering about libtensorflow (do we need it compiled with cuda as well?) and wondering about if we need to add Maybe I shall have a look at |
|
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 ( |
Contributor
|
It seems like:
|
Member
Author
|
Not much should have changed for the CPU builds, so I am not sure what's going on. My only hunch is that Azure's VMs are just different between the runs ... or they decreased the amount of available RAM? |
Contributor
|
Nice it seems that 2 builds pass. All seems normal on the CPU side! |
Contributor
|
Are you able to do the uploads to the conda-forge channel? I believe you've addressed all the comments from others. The next step to me is to:
If you can upload, then we can likely move forward. |
Member
Author
|
Argh, sorry, did this get lost again -.- :) Yes, I can do both -- uploads to conda-forge channel (I am a core member) and gather builds for Linux. |
Contributor
|
hmm that is a good point. I'll let you decide what you want to do. after this PR is done, we can work toward: |
Contributor
|
@wolfv would you be willing to work @183amir to test his CIs #144 (comment) |
Member
Author
Member
Author
Member
Author
|
Builds are running! :) |
Contributor
|
He has OSX-amd64 at least :/ Better than nothing. |
|
I am not running osx-arm64 cross compilation, it's running on a native machine and they fail: https://gitlab.idiap.ch/bob/conda/-/jobs/248772 |
Member
Author
|
So, I've finished all the builds (details in the collapsed section below). I am happy to upload these builds to the conda-forge channel (@xhochy / @h-vetinari can you merge & give the go on that?) @183amir happy to help in the other PR. I can also do some tests on a M1 to see what's going on. @hmaarrfk can you help with rebasing #144 once this one here is merged?! All build outputs!Variant: linux_64_cuda_compiler_version10.2cudnn7python3.7.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version10.2cudnn7python3.8.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version10.2cudnn7python3.9.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.0cudnn8python3.7.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.0cudnn8python3.8.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.0cudnn8python3.9.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.1cudnn8python3.7.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.1cudnn8python3.8.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.1cudnn8python3.9.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.2cudnn8python3.7.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.2cudnn8python3.8.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_version11.2cudnn8python3.9.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_versionNonecudnnundefinedpython3.7.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_versionNonecudnnundefinedpython3.8.____cpython
SHA256 sums Variant: linux_64_cuda_compiler_versionNonecudnnundefinedpython3.9.____cpython
SHA256 sums |
Member
Thanks a lot for all the builds, and for pu(bli)shing the playbook! I already LGTM'd the changes (aside from one open nit that I don't feel strongly about), but @xhochy still had an open review point (at least he hasn't responded to the comments since his original request), and I'm not sure if someone like @isuruf wants to chime in perhaps. So I'll hold off from merging just yet and wait a bit more if there's gonna be more feedback. |
Contributor
|
I can rebase #144. I've tried to centralize the open issues in the top comment. |
Member
Member
|
For the record, they seem to use a pretty broad definition of CVEs, including segfaults, crashes & null pointer exceptions. The descriptions of the CVEs don't seem to be public yet, nor their severity ratings. All in all I don't consider this a blocker, if we prepare #144 quickly after merging this. |
Contributor
|
Ok, I check that tensorflow is actually the last package for both the migrations in #144. We should finish this, solve the CVEs in 144. Otherwise we will be here for a while. It is somewhat challenging to rebase, so I'm worried that we will lose progress if we try to bite off too much. |
xhochy
requested changes
Nov 3, 2021
Member
xhochy
left a comment
xhochy
left a comment
There was a problem hiding this comment.
We probably want CPU packages preferred over GPU ones as in other packages where we are using track_features.
|
|
||
| cp cc_toolchain_config.bzl cc_toolchain_build_config.bzl | ||
| apply_cc_template cc_toolchain_config.bzl | ||
| apply_cc_template crosstool_wrapper_driver_is_not_gcc |
Member
There was a problem hiding this comment.
Fine with merging here but we should follow up on this soon.
|
|
||
| # weigh down cpu implementation and give cuda preference | ||
| track_features: | ||
| - tensorflow-cpu # [cuda_compiler_version == "None"] |
Member
There was a problem hiding this comment.
We did do this the other way around normally, esp @isuruf pointed out that we should give CPU builds a higher priority.
Member
Author
There was a problem hiding this comment.
I am following pytorch here ... https://github.com/conda-forge/pytorch-cpu-feedstock/blob/b8824904342b8ea142d297668bf0aa32790091a6/recipe/meta.yaml#L148-L149
The pip package also gives the GPU version by default.
Member
Author
There was a problem hiding this comment.
Also you'd only get the GPU version if there is actually a working CUDA driver installation on the system (the __cuda virtual package is required by cudatoolkit). In that case I think we can argue that it's also desired to get the GPU version by default.
Member
There was a problem hiding this comment.
Makes sense, I would still like to have @isuruf chime in here.
Member
There was a problem hiding this comment.
+1 for weighing down CPU, preferring GPU (where possible)
Member
xhochy
approved these changes
Nov 3, 2021
Member
|
@wolfv Feel free to merge and copy the builds to conda-forge! |
Member
Author
|
I am uploading the GPU builds now, first to my own channel, then to conda-forge! |
Member
Author
|
Builds are copied to the conda-forge channel: Thanks everyone! |
Member
|
I concur, thanks everyone! 🥳 |
6 tasks
Closed
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
0(if the version changed)conda-smithy(Use the phrase@conda-forge-admin, please rerenderin a comment in this PR for automated rerendering)Open discussion issues: