-
Notifications
You must be signed in to change notification settings - Fork 492
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
Add trigger for building Linux wheels #663
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/663
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 0659fdc with merge base 149e58e (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
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.
No major concerns, accepting to unblock
# NOTE: Binary build pipelines should only get triggered on release candidate builds | ||
# Release candidate tags look like: v1.11.0-rc1 |
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 don't understand this, aren't we also building nightlies here? (Sorry maybe this is a dumb question)
PACKAGE_NAME: torchtune | ||
PACKAGE_TYPE: wheel | ||
ARCH: x86_64 | ||
SMOKE_TEST_SCRIPT: "" |
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 think we had talked about potentially adding sanity checks of importing from the library or something like that here. Any plans to do that in this PR, or should we just do it later on?
SMOKE_TEST_SCRIPT: "" | ||
steps: | ||
- run: echo "Exposing env vars to all jobs" | ||
generate-matrix: |
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.
Any value to just adding a comment at the top of this job about which versions we should be building so folks don't have to go to the test-infra yaml to find this stuff?
ref: ${{ needs.setup-variables.outputs.REF }} | ||
setup-miniconda: true | ||
python-version: ${{ env.PYTHON_VERSION }} | ||
cuda-version: ${{ env.CU_VERSION }} |
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.
Do we need this if we're not enabling CUDA builds?
# shellcheck disable=SC1090 | ||
source "${BUILD_ENV_FILE}" | ||
# shellcheck disable=SC2086 | ||
${CONDA_RUN} python -m pip install build |
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.
So this will be supplanted by https://github.com/pytorch/test-infra/pull/5071/files? Will we be able to remove some of these other steps as well to rely directly on the test-infra version?
This uses the NOVA workflows from test-infra to build a Linux wheel.
Currently, I've set the ref to
pkg-for-wheel
for testing. This will need to be changed back before merging.