Skip to content
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

chore: bump python version #449

Merged

Conversation

dushyantbehl
Copy link
Contributor

@dushyantbehl dushyantbehl commented Jan 24, 2025

Description of the change

Related issue number

How to verify the PR

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

Signed-off-by: Dushyant Behl <dushyantbehl@in.ibm.com>
Copy link

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

@dushyantbehl dushyantbehl changed the title Bump python version chore: bump python version Jan 24, 2025
@github-actions github-actions bot added the chore label Jan 24, 2025
Copy link
Collaborator

@willmj willmj left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix and testing Dushyant! Apologies for the lack of understanding but could you clarify what prevents us from keeping a minimum version of 3.9 for users who may want to use it?

@dushyantbehl
Copy link
Contributor Author

Thanks for the quick fix and testing Dushyant! Apologies for the lack of understanding but could you clarify what prevents us from keeping a minimum version of 3.9 for users who may want to use it?

@willmj Anything less than v3.12 has a FIPS security compliance issue which stems from underlying python version...if security is not a concern we can keep multiple versions one internal to the product where we go with the compliant 3.12+ python else we keep it open to 3.9 users like we had already.

willmj
willmj previously approved these changes Jan 24, 2025
Copy link
Collaborator

@willmj willmj left a comment

Choose a reason for hiding this comment

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

That makes sense, thanks! This change seems good to me for now, but let's wait for @anhuong's approval before merging.

Comment on lines 14 to 15
- setup: "3.9"
tox: "py39"
- setup: "3.10"
tox: "py310"
- setup: "3.11"
tox: "py311"
- setup: "3.12"
tox: "py312"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should leave all versions to test unit tests on, we can still support python 3.9-3.12

@@ -17,7 +17,7 @@
ARG BASE_UBI_IMAGE_TAG=latest
ARG USER=tuning
ARG USER_UID=1000
ARG PYTHON_VERSION=3.11
ARG PYTHON_VERSION=3.12
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though our minimum version is set to 3.9, we have been building our image with 3.11. I don’t see an issue with moving this up to 3.12 except for the additional tests we should run to ensure everything works as expected. We can still have our minimum version be 3.9 but just has our automation running with 3.12. Currently our unit tests are the only thing that tests the suite of python version whereas our e2e tests focus solely on testing the image.

@willmj
Copy link
Collaborator

willmj commented Jan 27, 2025

Testing results on python 3.12 version:

3.12 Baseline runtime 3.12 Runtime with flags set 3.12 Baseline MF1 Score 3.12 MF1 score with flags set
Granite 3.1 8b FT 3257 3094 0.590 0.641
Granite 3.1 8b LoRA 1033 1043 0.610 0.605
Llama 3.1 8b FT 3482 2956 0.596 0.621
Llama 3.1 8b LoRA 1091 1092 0.577 0.568
PowerLM 3b QLoRA 1027 630 0.607 0.589

for comparison, this is what we saw on python 3.11

Baseline runtime Runtime with flags set Baseline MF1 Score MF1 score with flags set
Granite 3.1 8b FT 4476 3520 0.589 0.561
Granite 3.1 8b LoRA 1102 1094 0.599 0.601
Llama 3.1 8b FT 4066 3391 0.596 0.645
Llama 3.1 8b LoRA 1110 991 0.587 0.588

Signed-off-by: Anh Uong <anh.uong@ibm.com>
pyproject.toml Outdated Show resolved Hide resolved
anhuong and others added 2 commits January 27, 2025 12:40
Co-authored-by: Will Johnson <mwjohnson728@gmail.com>
Signed-off-by: Anh Uong <anhuong4444@gmail.com>
Copy link
Collaborator

@willmj willmj left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@anhuong anhuong left a comment

Choose a reason for hiding this comment

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

Updating python to 3.12 for fips compliance, image and library will be built with python 3.12 but support will remain for python 3.9-3.12

@aluu317 aluu317 merged commit 3daf5bb into foundation-model-stack:main Jan 27, 2025
9 checks passed
aluu317 added a commit that referenced this pull request Jan 27, 2025
chore: bump python version
Signed-off-by: Angel Luu <angel.luu@us.ibm.com>
@praveenj83
Copy link

Thanks everyone for the quick turnaround on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants