Skip to content

[VibeCoding] Convert architecture specific batch scripts to PowerShell #155807

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

Open
wants to merge 6 commits into
base: gh/malfet/398/base
Choose a base branch
from

Conversation

malfet
Copy link
Contributor

@malfet malfet commented Jun 12, 2025

Stack from ghstack (oldest at bottom):

  • Refactor common startup logic to common.ps1 (and check that exceptions can be propagated thru)
  • Source architecture specific scripts rather than call them as subprocesses

Using Claude.AI, see https://claude.ai/share/17e72de7-282b-431a-98e8-099286c804ea and https://claude.ai/share/54ae2b87-03b0-4589-9f4b-e19e99c08d06

cc @seemethere @osalpekar @atalman

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jun 12, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/155807

Note: Links to docs will display an error until the docs builds have been completed.

❌ 30 New Failures, 1 Unrelated Failure

As of commit 6a7e719 with merge base 2a3b41c (image):

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@malfet malfet added the topic: not user facing topic category label Jun 12, 2025
@malfet malfet requested a review from atalman June 12, 2025 15:40
@malfet malfet added the module: binaries Anything related to official binaries that we release to users label Jun 12, 2025
@atalman
Copy link
Contributor

atalman commented Jun 12, 2025

Looks like cuda126.ps1 and cuda129.ps1 is missing

@@ -0,0 +1,54 @@
# Import common functions
. "$PSScriptRoot\common.ps1"
Copy link
Contributor

Choose a reason for hiding this comment

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

can we try to rewrite it so that we don't need to add cuda12X.ps1 every time so that we can specify CUDA version as a param and reuse this script across all cuda verisons ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do it as followup...

Copy link
Contributor

@atalman atalman left a comment

Choose a reason for hiding this comment

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

Looks like this PR removes xpu.bat and cpu.bat

@atalman
Copy link
Contributor

atalman commented Jun 12, 2025

Also instead of ps1 can we try bash script or python ? This will minimize different scripts flavors we need to support. And will make it easier for multiple team members to support these scripts (more people are familiar with bash and python)

[ghstack-poisoned]
@malfet malfet requested a review from a team as a code owner June 12, 2025 17:57
@malfet malfet requested a review from atalman June 12, 2025 17:57
@malfet
Copy link
Contributor Author

malfet commented Jun 12, 2025

Looks like cuda126.ps1 and cuda129.ps1 is missing

Did you mean cuda128? (as 129 shouldn't be there, as it does not exists)

But yeah, fat fingers, added files now

@malfet malfet added ciflow/binaries Trigger all binary build and upload jobs on the PR and removed module: binaries Anything related to official binaries that we release to users labels Jun 12, 2025
@malfet
Copy link
Contributor Author

malfet commented Jun 12, 2025

Also instead of ps1 can we try bash script or python ?

Neither are native to Windows platform

Copy link
Contributor

@ZainRizvi ZainRizvi left a comment

Choose a reason for hiding this comment

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

Looks good. How are you testing the changes to ensure little bugs don't creep in? Is binary builds enough to ensure these get executed thoroughly?

[ghstack-poisoned]
@malfet malfet changed the title [VibeCode] Convert Batch scripts to PowerShell [VibeCoding] Convert architecture specific batch scripts to PowerShell Jun 12, 2025
[ghstack-poisoned]
@malfet
Copy link
Contributor Author

malfet commented Jun 12, 2025

Looks good. How are you testing the changes to ensure little bugs don't creep in? Is binary builds enough to ensure these get executed thoroughly?

Eyeballing + CI is the best I can hope for

malfet added 2 commits June 12, 2025 14:53
[ghstack-poisoned]
[ghstack-poisoned]
Copy link
Contributor

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/binaries Trigger all binary build and upload jobs on the PR Stale topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants