-
Notifications
You must be signed in to change notification settings - Fork 1k
Upgrade to Python 3.12 #1516
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 to Python 3.12 #1516
Conversation
Dockerfile.tmpl
Outdated
| # b/456239669: remove huggingface-hub pin when pytorch-lighting and transformer are compatible | ||
| # b/315753846: Unpin translate package, currently conflicts with adk 1.17.0 | ||
| RUN uv pip install --system --force-reinstall --no-deps torchtune gensim "scipy<=1.15.3" "huggingface-hub==0.36.0" "google-cloud-translate==3.12.1" | ||
| # b/(xxxxx): Unpin Pandas once cuml/cudf are compatible, version 3.0 causes issues |
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.
placeholder: we'll add bugs in one go once we stabilize changes.
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.
Before or after merging this 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.
i use to wait until we have a passing sgd (we use to test learn) to file all bugs
since we could end up needing to un/re-pinned packages
given we don't have a learn dependency anymore, i'll probably add them before merging.
i'm still looking into removing/unpinning more packages aside from these in a different PR, mostly I like waiting a bit before filing a bunch of bugs.
djherbis
left a comment
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.
Why do we need the massive assets added?
|
@djherbis our "tests/test_keras_nlp.py" test requires them. given our test are offline, we use a mock server and provide the files. the older version uses bert_tiny_en_uncased version 2, this new one is requesting version 3. we'll need to remove version 2 in a follow up. if we want to remove the test due to size, we can, otherwise this is needed |
|
Any reason for not removing the version 2 of |
| "nvidia-nvjitlink-cu12==12.5.82" | ||
| RUN uv pip install --system --force-reinstall "pynvjitlink-cu12==0.5.2" | ||
|
|
||
| # b/385145217 Latest Colab lacks mkl numpy, install it. |
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.
In the PR description, you mention we agreed on removing mkl numpy, do you have a link to this discussion?
Mostly for future reference and my own curiosity.
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.
created a high level bug for the python 3.12 and posted it in their. i tend to avoid posting internal links here (other than bugs): https://b.corp.google.com/issues/468103319
removing mkl numpy is an option for unblocking numpy 2.0 given that we been stuck on 1.26.4 for a while.
Now the new base image (py3.12) requires we have 2.2.2.
we currently install mkl from this index, but doesn't provide anything pass 1.26.4: https://pypi.anaconda.org/intel/simple/numpy/
newer versions can be found in this repo, but unfortunately doesn't have a 2.2.2 version which this new base image really needs (as in many package break): https://software.repos.intel.com/python/pypi
rosbo
left a comment
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.
Thanks for the clarification and good work!
Its fine for now I guess, just generally doesn't feel great to commit a lot of extra data into the repo. |
we're bumping our image to Python 3.12
which required the following:
remove numpy-mkl: unfortunately we were not able to find/install a compatible version, we opted to remove it based on previous conv we had on this topic. we will instead use the default installed in colab.
remove cuml installation hack:
thankfully are able to use the pre-installed base image version without build errors.
unpinned package due to learn:
Learn is no longer dependent on this build, we can freely unpinned many packages
-seaborn, scikit-learn, matplotlib, geopandas, TPOT, shapely, tfdf, ydf, etc
remove incompatible packages:
Some of these are no longer support and cause build issues
-pydegensac, pymc3, eli5, etc
remove preinstalled package:
where applicable we removed packages that are already installed in colab base image
https://b.corp.google.com/issues/468103319