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

NEW: Add default values for cuda_compiler on osx and windows #27547

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

carterbox
Copy link
Member

@carterbox carterbox commented Sep 11, 2024

Fixing broken feedstock rendering from #26580

@beckermr, is this what you mean? The pinnings here always lag behind the channel-wide pinnings.

https://github.com/conda-forge/admin-requests/actions/runs/10816201902

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

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

I was trying to look for recipes to lint for you, but couldn't find any.
Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

@carterbox carterbox self-assigned this Sep 11, 2024
Copy link

Hi! Thanks for your contribution to conda-forge.

We appreciate your effort in improving our project.

However, it looks like some changes were made outside the recipes directory. To ensure everything runs smoothly, please make sure that recipes are only added to the recipes folder and no other files are changed. Further, please make sure that any changes are reverted before you submit your pull request for review.

If these changes are intentional (and you aren't submitting a recipe), please attach a maintenance label to the PR.

Thanks!

@carterbox
Copy link
Member Author

Or did you want something like this:

{% if cuda_compiler_version in (None, "None", True, False) %}
{% set cuda_major = 0 %}
{% else %}
{% set cuda_major = environ.get("cuda_compiler_version", "11.8").split(".")[0] | int %}
{% endif %}

added to the recipes?

@beckermr
Copy link
Member

we should ask @jakirkham or someone who knows the cuda stack better on @conda-forge/core

@carterbox
Copy link
Member Author

I'm going to merge now and ask for @jakirkham's opinion later because I believe he's out of the office.

@carterbox carterbox merged commit ac89d0d into conda-forge:main Sep 11, 2024
5 of 6 checks passed
@isuruf
Copy link
Member

isuruf commented Sep 11, 2024

cuda_compiler_version is set https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/main/recipe/conda_build_config.yaml#L56-L57. I don't understand why this is necessary.

@beckermr
Copy link
Member

@beckermr
Copy link
Member

This might be the wrong solution, but this is what we are trying to fix.

@isuruf
Copy link
Member

isuruf commented Sep 11, 2024

The correct way is https://github.com/conda-forge/pytorch-cpu-feedstock/blob/main/recipe/meta.yaml#L74

@carterbox
Copy link
Member Author

IMHO, we shouldn't have to do that because this recipe is CUDA only, so the cuda compiler version should be defined.

@beckermr
Copy link
Member

We should separate the issues out. Let's push an immediate fix to unblock staged recipes by using the current way cuda is done. We can discuss changes to how cuda is done as another issue.

@jakirkham
Copy link
Member

Sorry for the slow reply. As Daniel noted I was out for some time

Likely the issue that was encountered is the same one we have seen elsewhere with conda-build version 24.7 (not in 24.5 or earlier): conda-forge/conda-smithy#2011

Recently Michael fixed this error in PR ( conda/conda-build#5458 ), which was included in conda-build version 24.9.0

Have tried re-rendering both recipes locally using PR ( #26580 )'s merge commit ( 8d2e5e3 ) and conda-build version 24.9.0. This works correctly locally

@jakirkham jakirkham mentioned this pull request Sep 24, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants