Skip to content

Add pytorch-cpu-feedstock to the cirun-macos-m4-large runner#119

Merged
jaimergp merged 2 commits into
conda-forge:mainfrom
mgorny:pytorch-mac
Nov 8, 2025
Merged

Add pytorch-cpu-feedstock to the cirun-macos-m4-large runner#119
jaimergp merged 2 commits into
conda-forge:mainfrom
mgorny:pytorch-mac

Conversation

@mgorny
Copy link
Copy Markdown
Contributor

@mgorny mgorny commented Nov 6, 2025

I've assumed we want to keep the policy separate from pytorch-cpu-feedstock-windows-policy.

CC @h-vetinari

Copy link
Copy Markdown
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

I've assumed we want to keep the policy separate from pytorch-cpu-feedstock-windows-policy.

TBH, I think we should be using one policy per feedstock, unless there's really strong reasons why we need different set of users to have access to specific machines (e.g. linux vs. win vs. macos)

Feel free to rename the policy to something better, I did the same in f6d8a19 for example

I've assumed we want to keep the policy separate from
`pytorch-cpu-feedstock-windows-policy`.

Signed-off-by: Michał Górny <mgorny@quansight.com>
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented Nov 6, 2025

The current pytorch-cpu-feedstock-policy and pytorch-cpu-feedstock-windows-policy have different access lists. Should I keep them separate, i.e. use the "windows" list for windows+macOS, or do we want to merge them somehow?

@mgorny mgorny force-pushed the pytorch-mac branch 2 times, most recently from e0e8d44 to 4540921 Compare November 6, 2025 17:27
Combine the two Windows/macOS policies into a single `cpu` policy,
and rename the open-gpu server policy into `gpu`.

Signed-off-by: Michał Górny <mgorny@quansight.com>
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented Nov 6, 2025

For now, left them split into gpu (for open-gpu-server) and cpu (for Windows and macOS). Lemme know if you prefer them merged somehow.

@jaimergp
Copy link
Copy Markdown
Member

jaimergp commented Nov 6, 2025

Requests for addition are usually done via admin-requests for higher visibility. Example: https://github.com/conda-forge/admin-requests/blob/main/examples/example-open-gpu-server.yml

@h-vetinari
Copy link
Copy Markdown
Member

I would prefer merging access lists (so we don't have extra overhead that we need several maintainers to start each job), but it's not critical for me, as long as the core maintainers of the pytorch feedstocks all have access to all resources.

I'll let @jaimergp guide you how to pass through admin-requests if necessary, I don't mind how that part is done.

@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented Nov 7, 2025

Requests for addition are usually done via admin-requests for higher visibility. Example: https://github.com/conda-forge/admin-requests/blob/main/examples/example-open-gpu-server.yml

Sure, will file one.

I would prefer merging access lists (so we don't have extra overhead that we need several maintainers to start each job), but it's not critical for me, as long as the core maintainers of the pytorch feedstocks all have access to all resources.

Well, I'm fine either way — my assumption was that we want the ACL for GPU controlled at Quansight level, and FWICS not all feedstock maintainers are there, so presumably we'd cut off Windows access from some.

mgorny added a commit to mgorny/admin-requests that referenced this pull request Nov 7, 2025
xref conda-forge/.cirun#119

Signed-off-by: Michał Górny <mgorny@quansight.com>
@h-vetinari
Copy link
Copy Markdown
Member

conda-forge/admin-requests#1743 did not create a new PR here (see also conda-forge/admin-requests#1744). I'm not very interested in being a stickler for procedure here, which for now simply isn't working yet.

Well, I'm fine either way — my assumption was that we want the ACL for GPU controlled at Quansight level, and FWICS not all feedstock maintainers are there, so presumably we'd cut off Windows access from some.

Obviously, this shouldn't regress people's capabilities. What I meant is that people pushing to the feedstock (especially the core maintainers) absolutely need to be able to trigger all different resources with a single commit. To me it'd make most sense to have one list for that, but ultimately I don't care enough to argue about this if someone has a different opinion.

Comment thread .access.yml
pull_request: true
users_from_json: https://raw.githubusercontent.com/Quansight/open-gpu-server/main/access/conda-forge-users.json
- id: pytorch-cpu-feedstock-policy
- id: pytorch-cpu-feedstock-gpu-policy
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

gpu-policy is also misnamed IMO, because we do both GPU and CPU-only builds on that server.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would open-gpu work better? :-)

Copy link
Copy Markdown
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

I now think we should do #121

Copy link
Copy Markdown
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Let's unblock adding pytorch to the macos runners here first, then we can discuss whether we unify policies or not (two different matters).

@jaimergp jaimergp merged commit 2bdf7b5 into conda-forge:main Nov 8, 2025
1 check passed
@mgorny mgorny deleted the pytorch-mac branch November 8, 2025 16:06
@mgorny
Copy link
Copy Markdown
Contributor Author

mgorny commented Nov 8, 2025

Thanks! I'm going to try building PyTorch now.

@h-vetinari
Copy link
Copy Markdown
Member

I was at a wedding all day yesterday and had no time to respond, sorry. I had just removed my hold so that @mgorny could hopefully make progress (while we figure out the situation in #122). However, the name pytorch-cpu-feedstock-cpu-policy is really an abomination IMO. Aside from the double "cpu", we do have GPUs on the windows server too. If #122 takes longer, then we need to rename this, for example

-pytorch-cpu-feedstock-gpu-policy
+pytorch-feedstock-linux-policy
 ...
-pytorch-cpu-feedstock-cpu-policy
+pytorch-feedstock-win-macos-policy

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.

3 participants