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

restrict host compiler version based on CUDA version #55

Closed
wants to merge 3 commits into from

Conversation

kkraus14
Copy link
Contributor

@kkraus14 kkraus14 commented Dec 8, 2020

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.

Closes #51

@conda-forge-linter
Copy link

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.

@kkraus14
Copy link
Contributor Author

kkraus14 commented Dec 8, 2020

@conda-forge-admin, please rerender

Comment on lines +33 to +37
run_constrained:
- gcc_linux-64 <=10 # [linux and (cuda_compiler_version or "").startswith("11.1.")]
- gcc_linux-64 <=9 # [linux and (cuda_compiler_version or "").startswith("11.0.")]
- gcc_linux-64 <=8 # [linux and (cuda_compiler_version or "").startswith(("10.2.", "10.1."))]
- gcc_linux-64 <=7 # [linux and (cuda_compiler_version or "").startswith(("10.0.", "9.2."))]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using run constraints felt more appropriate than run dependencies to continue to allow people to bring their own compiler

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will work as the version is already set in the recipe's build variants, which conflicts.

My guess is we will need to use zip_keys with the cxx_compiler_version in conda-forge-pinning like what we have done already with docker_image. Though we can test this hypothesis out by adding a conda_build_config.yaml to the recipe here with the same change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit lost as to how I would go about doing this. Would I create a new key with say [9, 7, 7, 7, 9] to match the cuda_compiler_version keys? How would I make {{ compiler("c") }} play nicely with that since it seems to already be set by the earlier zip key?

Copy link
Member

Choose a reason for hiding this comment

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

I didn't manage to get that particular zip working: conda-forge/conda-forge-pinning-feedstock#1000

@kkraus14
Copy link
Contributor Author

Closing as this approach won't work. Will open a PR in conda-forge-pinnings.

@kkraus14 kkraus14 closed this Dec 11, 2020
@jakirkham
Copy link
Member

Sounds good.

Yeah to your earlier question, we would need to combine them into the same zip_keys group. Shouldn't be too difficult. We can also build a migrator on the same premise and test that out here as well ( #47 ). That should help us build confidence that it is doing the right thing.

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 this pull request may close these issues.

nvcc package should constrain c / cxx compiler versions based on cuda version
4 participants