Block on Python 3.13 version #1899
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/1899
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit ffcf4f1 with merge base 74139c9 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
SalmanMohammadi
approved these changes
Oct 24, 2024
|
OK this is pretty impressive. Overall it looks reasonable, one thing I don't see is how you tested it though. Did you manually trigger the build job to confirm everything works now? |
ebsmothers
approved these changes
Oct 25, 2024
Comment on lines
+31
to
+58
| filter-python-versions: | ||
| needs: generate-matrix | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| matrix: ${{ steps.set-matrix.outputs.matrix }} | ||
| steps: | ||
| - name: Filter matrix to exclude Python 3.13 | ||
| id: set-matrix | ||
| shell: python | ||
| env: | ||
| input-matrix: ${{ needs.generate-matrix.outputs.matrix }} | ||
| run: | | ||
| import os | ||
| import json | ||
|
|
||
| # Grab environment variables | ||
| input_matrix = json.loads(os.environ["input-matrix"]) | ||
| github_output_file = os.environ["GITHUB_OUTPUT"] | ||
|
|
||
| # Filter out any builds for 3.13 | ||
| filtered_matrix = {"include": []} | ||
| for build in input_matrix["include"]: | ||
| if build["python_version"] != "3.13": | ||
| filtered_matrix["include"].append(build) | ||
|
|
||
| # Write the new matrix to the default outputs file | ||
| with open(github_output_file, "w") as handle: | ||
| handle.write(f"matrix={json.dumps(filtered_matrix)}") |
thomasjpfan
added a commit
to thomasjpfan/torchtune
that referenced
this pull request
Nov 12, 2024
This reverts commit 8e013c2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The worst hack I've written so far in my life. Github Actions is a (useful) abomination.
Goal: Don't run nightly builds on Python 3.13. We depend on HF Datasets, which depends on PyArrow, who haven't released a Python 3.13 wheel. It is expected to come out soon in version v18. Therefore, Python 3.13 builds break.
Simple enough, right?
WRONG
SO WRONG
UNBELIEVABLY WRONG.
Attempt 1: Figure out a way to make the
buildjob conditional on whether the matrix entry way for Python 3.13. Eight hours later, it's pretty clear that this isn't possible (through torchtune) b/c thebuildjob is controlled by dev-infra and we just pass in the entire matrix and it handles the actual execution.Attempt 2: Filter the matrix generated by the
generate-matrixjob to remove all Python 3.13 entries. It was about three hours in that I figured out I could create a Python shell in the filtering job. This sped things up and after another couple hours, here we are. But, Joe, how do you know it works if Python 3.13 jobs are only kicked off for nightly builds and you've done all your testing on this PR? I toggled the filtering function to exclude Python 3.9 jobs (since those are the only ones kicked off on PRs) and confirmed that these jobs were not passed down to thebuildjob. Is that good enough? No probably not, but what are you going to do about it?Final question - Was this a good use of my time?: Probably not. PyArrow will be out soon and we could've just had broken nightlies for Python 3.13 until then b/c no one is really using Python 3.13. But I know that my teammates will express great admiration and gratitude for making the little box green instead of red and that makes it alllllll worth it.