-
Notifications
You must be signed in to change notification settings - Fork 396
Upgrade dependencies (now supports and defaults to Python 3.12 and Ubuntu 24.04) #1826
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
Upgrade dependencies (now supports and defaults to Python 3.12 and Ubuntu 24.04) #1826
Conversation
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.
Thank you @SamuelMarks. I assume we will also need to make changes in constraints_gpu.txt
and requirements_with_jax_ai_image.txt
?
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.
Is it possible to test this setup with a MaxText workload/base model?
@bvandermoon Sure happy to update those also. @shralex was talking about making 3.12 the new default. How about this:
(not sure the process for upgrading Can upgrade them all in this PR or separate it out into multiple. Let me know 🤗 |
I tried building a maxtext image by forking this pr and found that the build fails when using MODE=stable or MODE=nightly. The commands I ran were
The build fails due to this line in setup.sh. We are pinning a version of setuptools that is incompatible with python 3.12. I was able to get an image built successfully by upgrading the setuptools version with this command:
@gobbleturk would it be fine to upgrade setuptools since it was pinned initially to fix the bug in cloud documentation: b/402501203 |
@Rohan-Bierneni Interesting. Yeah in #1954 I explicitly upgrade |
4bc1b53
to
7d8ff4f
Compare
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.
Thank you @SamuelMarks. Please confirm there won't any recent requirements/dependency changes that need to be updated/rebased here too.
Have you been able to test the Docker images still build, MaxText still trains on a real TPU, etc.? The unit tests are good for a lot of that but they only run one of the images we provide. Please run these tests before merging if you haven't already
For the file requirements_with_jax_stable_stack_0_6_1_pipreqs.txt, shouldn't we leave the setuptools pin as is since it was tested that it is compatible with 0.6.1. Essentially all the deps in that file should be pinned for reproducibility with jax 0.6.1. |
I don't think it needs that pinning. It doesn't break 3.10, 3.11, 3.12. We should, IMHO, make the dependencies as widely supporting of underlying CPython version as possible. |
Sounds good that makes sense |
14047e1
to
db4d4f9
Compare
92bc830
to
0e1c6f1
Compare
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.
LGTM. Manually adding pull_ready tag since old CPUTests check is overriden in pr with new configuration. This causes a gap in the pr checklist where old test is never ran while new test passes. Once pr merges, this change should be fixed. Ref: https://screenshot.googleplex.com/s4M6YpsecC3UNrJ
48e38f4
to
c5ca6c7
Compare
c5ca6c7
to
13543e8
Compare
86be2a6
into
AI-Hypercomputer:main
Description
Upgrade dependencies
python3 -m pip install -r requirements.txt
with no special flags(I also sorted the dependencies so it's easier to debug and add/remove deps in the future)
FWIW Python 3.13 cannot yet be supported, but this patch did make the dependencies installable:
Tests
N/A
Checklist
Before submitting this PR, please make sure (put X in square brackets):