-
Notifications
You must be signed in to change notification settings - Fork 16
recreate wheel build PR #288
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
Conversation
.github/packaging/pre_build_gpu.sh
Outdated
# their wheels into dist/. | ||
|
||
MONARCH_COMMIT="265034a29ec3fb35919f4a9c23c65f2f4237190d" | ||
TORCHTITAN_COMMIT="82f0287b966f1735819a377a9a09e7a303c55faa" |
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.
nightly?
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.
This is actually not used. I think we really only need to successfully upload Monarch and vLLM here, the rest can be handled elsewhere
this looks fun |
WHL_DIR="${GITHUB_WORKSPACE}/wheels/dist" | ||
DIST=dist/ | ||
|
||
echo "Uploading wheels to S3" |
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.
should we change this to like "Here are the files we're uploading to S3" or remove altogether?
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.
Yeah let me make this a bit clearer
pip wheel -v --no-build-isolation --no-deps . -w "$WHL_DIR" | ||
} | ||
|
||
build_vllm No newline at end of file |
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.
oh I wonder if we need to do the append_date here too. Which forge wheel ultimately gets uploaded?
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.
append_date is updating the forge wheel's name, right? Do we even care about that? (Actually if we are running this nightly shouldn't we be appending date to monarch and vllm wheels?)
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.
I guess that's true on both accounts. I dunno I'm not too opinionated
.github/workflows/build_wheels.yaml
Outdated
|
||
jobs: | ||
build: | ||
# if: github.repository_owner == 'pytorch' |
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.
# if: github.repository_owner == 'pytorch' | |
if: github.repository_owner == 'pytorch' |
.github/workflows/build_vllm.yaml
Outdated
name: Build pinned vLLM against PyTorch nightly and upload | ||
|
||
on: | ||
pull_request: |
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.
should we remove pull_request
? I assume we don't want this to actually run all the builds per PR
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.
Oh yeah good call, thank you
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
=======================================
Coverage ? 61.62%
=======================================
Files ? 73
Lines ? 7130
Branches ? 0
=======================================
Hits ? 4394
Misses ? 2736
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Need to recreate this PR due to issues after the repo being made public. Only change from the previous version is the addition of the cp command at the end of the post-build script