-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
try v1.12 #118
try v1.12 #118
Conversation
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 ( |
@conda-forge-admin, please rerender. |
…nda-forge-pinning 2022.06.13.22.29.59
@conda-forge-admin, please rerender |
@conda-forge-admin, please rerender. |
…nda-forge-pinning 2022.06.13.22.29.59
Never noticed this before...
|
@hmaarrfk this is the
|
This is likely the issue: pytorch/pytorch#75398 Why don't we pull https://github.com/conda-forge/onednn-feedstock/blob/main/recipe/meta.yaml? |
I'm not sure what we are pulling. Generally this recipe is hard to build. So for many reasons we don't break it up. |
I have no idea, it seems the issue is that these |
I will try to see what it would take for us to simply use the onednn feedstock instead of cloning the other. |
(will reopen another PR later) |
…nda-forge-pinning 2022.06.22.11.09.47
…nda-forge-pinning 2022.06.24.18.35.54
…nda-forge-pinning 2022.06.29.18.34.42
@@ -1,4 +1,7 @@ | |||
azure: | |||
settings_osx: | |||
pool: | |||
vmImage: macos-12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isuruf, is this safe? The advantage of setting macos-12 here is to get the highest sdks and that enables MPS by default. However, I could see this being problematic breaking builds for macos < 12. What do you think?
There should be an alternative way where we can force it to build for MPS, but the logic upstream is pretty inconvenient: https://github.com/pytorch/pytorch/blob/b370959da11fe1c5fe9a6a6335a9b10ab6ad0a39/CMakeLists.txt#L90-L124. We can potentially just patch this and force it to build, but that may be risky.
That's why I initially opted to separate them. Keep the status quo for our regular build and add a highly constrained MPS build with the "MPS"/"Metal" tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note, we don't have SDKs >= 12 in conda-forge. I submitted (and closed) a PR for that initially before deciding it was easier to just split them. Here's the PR: conda-forge/conda-forge-ci-setup-feedstock#201
these issues persist, closing |
The goal of this dev PR is:
xref #107
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)