FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Moving from Python3.8 to Python 3.11 by init-22 · Pull Request #811 · mlcommons/algorithmic-efficiency · GitHub

/ algorithmic-efficiency Public

Moving from Python3.8 to Python 3.11 - #811

Merged
priyakasimbeg merged 29 commits into
mlcommons:python_upgradesfrom
init-22:python311
Dec 20, 2024
ConversationCommits (29)ChecksFiles changed
Merged

Moving from Python3.8 to Python 3.11#811
priyakasimbeg merged 29 commits into
mlcommons:python_upgradesfrom
init-22:python311

Conversation

init-22 commented Nov 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Making the following major changes:

  • Python 3.11.10
  • Jax 0.4.35
  • Pytorch 2.5.1
  • Tensorflow 2.18

#805

init-22 requested a review from a team as a code owner November 15, 2024 13:45

github-actions Bot commented Nov 15, 2024 •
edited
Loading

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

priyakasimbeg changed the base branch from main to dev November 21, 2024 19:00
priyakasimbeg changed the base branch from dev to main November 22, 2024 05:26

priyakasimbeg commented Nov 22, 2024

Copy link
Copy Markdown
Contributor

temporarily changed target branch to main to trigger regression tests.

priyakasimbeg changed the base branch from main to dev November 22, 2024 05:28

init-22 commented Nov 22, 2024

Copy link
Copy Markdown
Contributor Author

Some of the required package versions are only available in python3.11 and the tests are using python3.9 it seems, we might need to change that

priyakasimbeg commented Nov 25, 2024

Copy link
Copy Markdown
Contributor

Could you update the tests to use 3.11 in this PR?

init-22 added 14 commits November 30, 2024 13:07
fix: changing the python versions in workflow to pass the tests
1bc2a7b
fix: changing numpy compatible version
7a0fee3
adding key_data to check the CI tests
7cdea16
fix: updated packge of sacrebleu changed the way it used to work, hen…
7264c3f 
…ce using the corpus_bleu from the main package
fix: temporarily commenting tfa
abbdc82
fix: explicitly using mask kwarg to use MultiHeadDotProductAttention …
86029a7 
…and also using sacrebleu
fix: using flax.core.pop instead of variables.pop, better way to upda…
aca45a2 
…te batch_stats
fix: changing the traindiffs_tests branch to main again
2618c5e
fix: unfreeze() in test_param_shapes expect FrozenDict also added fla…
8c90625 
…x.core.pop instead of variables.pop
fix: formatting changes with yapf
1b587b7
fix: running yapf again with 0.32, earlier using 0.43
c65d93e
fix: running yapf again with 0.32, earlier using 0.43
3afd1df
fix: latest versions of typing dont support Text instead str is recom…
6ff2010 
…mended
fix: minor yapf
55bacbd

priyakasimbeg commented Dec 5, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey Isaac,
Could you paste the output of pip freeze in this PR as well of an environment setup with these upgrades?

Also I think we want to disable linting check in the lines that are failing the pylint tests.

priyakasimbeg changed the base branch from dev to main December 5, 2024 18:24
priyakasimbeg changed the base branch from main to dev December 5, 2024 18:25
runame self-requested a review December 5, 2024 18:48
priyakasimbeg changed the base branch from dev to main December 6, 2024 00:19

priyakasimbeg commented Dec 6, 2024

Copy link
Copy Markdown
Contributor

Running Containerized regression tests: https://github.com/mlcommons/algorithmic-efficiency/actions/runs/11934647960

priyakasimbeg changed the base branch from main to python_upgrades December 6, 2024 00:28

priyakasimbeg commented Dec 6, 2024

Copy link
Copy Markdown
Contributor

@init-22 I'm going to merge this in to a new branch mlcommons/python_upgrades so that I can trigger the tests. Currently the containerized tests on our self-hosted runners can only run branches that are on this repo. Could you pull that branch and make follow up changes on there?

runame reviewed Dec 6, 2024
View reviewed changes

runame left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What was the reason for switching to sacrebleu?

I haven't reviewed the Dockerfile and all the version updates, the rest LGTM.

Comment thread algorithmic_efficiency/workloads/imagenet_resnet/imagenet_jax/randaugment.py Outdated
Comment thread algorithmic_efficiency/random_utils.py
Comment thread algorithmic_efficiency/workloads/wmt/wmt_jax/workload.py Outdated

init-22 commented Dec 7, 2024

Copy link
Copy Markdown
Contributor Author

@priyakasimbeg Please check the output of pip freeze

pip_freeze_python311.txt

runame approved these changes Dec 19, 2024
View reviewed changes

runame left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, thanks for porting the custom tf addons!

Maybe we can get rid of the sacrebleu dependency in a follow up PR (to avoid blocking this PR further).

Comment thread algorithmic_efficiency/workloads/imagenet_resnet/imagenet_jax/randaugment.py Outdated
priyakasimbeg merged commit 9d1c957 into mlcommons:python_upgrades Dec 20, 2024
github-actions Bot locked and limited conversation to collaborators Dec 20, 2024
This file contains hidden or 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
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

priyakasimbeg priyakasimbeg left review comments
runame runame approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL