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

Determining CUDA Docker image/compiler version #237

Closed
jakirkham opened this issue Jun 10, 2019 · 17 comments · Fixed by #285
Closed

Determining CUDA Docker image/compiler version #237

jakirkham opened this issue Jun 10, 2019 · 17 comments · Fixed by #285

Comments

@jakirkham
Copy link
Member

We recently added a few Docker images based off of Docker images from NVIDIA. Also we have the beginnings of a shim package to use nvcc. The next thing to consider is how we can use these in conjunction with each other. Am curious if anyone has thoughts on how we might do this.

@scopatz
Copy link
Member

scopatz commented Jun 10, 2019

Should nvcc be a CDT?

@jakirkham
Copy link
Member Author

Why should it be a CDT? It should be a compiler, no?

@scopatz
Copy link
Member

scopatz commented Jun 14, 2019

Oh sure it should be a compiler, but we can't redistribute nvcc so isn't a CDT the right way to declare an external dependency?

@jakirkham
Copy link
Member Author

This is the problem that the nvcc shim package is solving for us. ( conda-forge/staged-recipes#8229 )

It's similar to how we handle the Visual Studio compiler today. IOW we rely on some VM images to have the compiler (in this case we have Docker images) and we do the bare minimum to explain to conda-build that there is a compiler in that image and how to use it.

Does that make sense?

@jakirkham
Copy link
Member Author

To clarify, here's the question I'm thinking about: How do we select the image based on the version of CUDA we want to build against?

With Windows we have gotten away with using the same image for all compilers, but we are unable to do this with nvcc as we need different images for each version of the compiler.

The only case where we have needed multiple images before was the compiler migration. We could borrow this logic for handling nvcc. Maybe this is the way we should solve this problem? Or is there another way we should solve this?

@scopatz
Copy link
Member

scopatz commented Jun 18, 2019

I think I see, like we need an {{ nvcc() }} function that returns the proper package for a version of nvcc

@jakirkham
Copy link
Member Author

So {{ compiler("nvcc") }} works correctly and does that with PR ( conda-forge/staged-recipes#8229 ).

What we still need is a way to turn a recipe using {{ compiler("nvcc") }} into a feedstock that builds using Docker images (condaforge/linux-anvil-cuda:9.2 and condaforge/linux-anvil-cuda:10.0). This probably needs to use zip_keys to tie {{ compiler("nvcc") }} versions to Docker image versions. Though there is probably more than that required.

@jakirkham
Copy link
Member Author

@jjhelmus, do you have any thoughts on how we should handle this?

@isuruf
Copy link
Member

isuruf commented Jun 25, 2019

zip_keys should work. conda-smithy uses docker_image variable in conda_build_config.yaml to generate CI files

@jakirkham
Copy link
Member Author

cc @mariusvniekerk (as we discussed this last Friday)

@jakirkham
Copy link
Member Author

zip_keys should work. conda-smithy uses docker_image variable in conda_build_config.yaml to generate CI files

I guess the challenge that Marius and I were discussing is that sometimes we have nvcc and sometimes we don't. In the case we don't we want to fallback to usual Docker image. In the case we do we want to add one of the CUDA-based Docker images. How do we handle this?

@jakirkham
Copy link
Member Author

@isuruf, do you know if staged-recipes also respects the docker_image variant?

@isuruf
Copy link
Member

isuruf commented Aug 15, 2019

I guess the challenge that Marius and I were discussing is that sometimes we have nvcc and sometimes we don't. In the case we don't we want to fallback to usual Docker image. In the case we do we want to add one of the CUDA-based Docker images. How do we handle this?

something like,

cuda_version:
  - 9
  - 10
  - None
docker_image:
  - condaforge/linux-anvil-cuda9
  - condaforge/linux-anvil-cuda10
  - condaforge/linux-anvil
zip_keys:
  - - docker_image
    - cuda_version

and you can skip cuda_version == None explicitly in the recipe. (Or have conda-smithy do it for you)

@isuruf
Copy link
Member

isuruf commented Aug 15, 2019

@isuruf, do you know if staged-recipes also respects the docker_image variant?

No, it doesn't. You'll have to run docker within docker to do that.

@isuruf
Copy link
Member

isuruf commented Aug 15, 2019

docker_image is already in a zip_keys which makes this complicated.
Here's another approach,

In conda-smithy

diff --git a/conda_smithy/configure_feedstock.py b/conda_smithy/configure_feedstock.py
index 3e09f2e..5df3fb6 100644
--- a/conda_smithy/configure_feedstock.py
+++ b/conda_smithy/configure_feedstock.py
@@ -305,6 +305,7 @@ def _collapse_subpackage_variants(list_of_metas, root_path):
     }
     all_used_vars.update(always_keep_keys)
     all_used_vars.update(top_level_vars)
+    all_used_vars.add("docker_image_cuda")
 
     used_key_values = {
         key: squished_input_variants[key]
@@ -322,6 +323,9 @@ def _collapse_subpackage_variants(list_of_metas, root_path):
     _trim_unused_zip_keys(used_key_values)
     _trim_unused_pin_run_as_build(used_key_values)
 
+    if "cuda_compiler" not in used_key_values:
+       used_key_values.pop("docker_image_cuda", None)
+
     # to deduplicate potentially zipped keys, we blow out the collection of variables, then
     #     do a set operation, then collapse it again
 
@@ -365,8 +369,11 @@ def finalize_config(config, platform, forge_config):
     """For configs without essential parameters like docker_image
     add fallback value.
     """
-    if platform.startswith("linux") and not "docker_image" in config:
-        config["docker_image"] = [forge_config["docker"]["fallback_image"]]
+    if platform.startswith("linux"):
+        if "docker_image" not in config:
+            config["docker_image"] = [forge_config["docker"]["fallback_image"]]
+        elif "docker_image_cuda" in config:
+            config["docker_image"] = [str(config["docker_image_cuda"][0])]
     return config

In conda_build_config.yaml

docker_image_cuda:
  - condaforge/linux-anvil-cuda:10.0
  - condaforge/linux-anvil-cuda:9.2
cuda_compiler:
  - nvcc
cuda_compiler_version:
  - 10.0
  - 9.2
zip_keys:
  -
    - cuda_compiler_version
    - docker_image_cuda

In recipe/meta.yaml

requirements:
  build:
    - {{ compiler('cuda') }}

@jaimergp
Copy link
Member

Hi! Thanks for all the great work here! I am looking forward to use this in several upcoming recipes that need GPU support.

This is slightly unrelated since it's not about Docker images but, how are you planning to bring GPU capabilities to Windows and OS X? Through separate Azure Pipelines yml files that manually install the Nvidia drivers?

@jakirkham
Copy link
Member Author

Thanks for taking a look @isuruf! 😄

and you can skip cuda_version == None explicitly in the recipe. (Or have conda-smithy do it for you)

This None trick is an interesting idea.

No, it doesn't. You'll have to run docker within docker to do that.

Hopefully we can come up with a better solution than this. 😉

docker_image is already in a zip_keys which makes this complicated.
Here's another approach,

Interesting. So you would propose using either of two different variants for the Docker image based on whether or not CUDA is used?

It's worth noting that some packages will have CPU and GPU builds. So we probably want some way to have both here and select between them based on the build. Will give that some more thought.

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 a pull request may close this issue.

4 participants