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

[Fix] some pkg versions for the Tensorflow ImageStream manifest #742

Merged

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Oct 11, 2024

this fixes versions of the CUDA bundle and Numpy packages

This is a followup for the #721. This comment exactly.

Description

How Has This Been Tested?

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

this fixes versions of the CUDA bundle and Numpy packages
@jstourac jstourac changed the title [Fix] the version of the CUDA bundle for the Tensorflow ImageStream manifest [Fix] some pkg versions for the Tensorflow ImageStream manifest Oct 11, 2024
@jstourac jstourac marked this pull request as ready for review October 11, 2024 11:18
@openshift-ci openshift-ci bot requested a review from paulovmr October 11, 2024 11:18
@jiridanek
Copy link
Member

/lgtm

@atheo89
Copy link
Member

atheo89 commented Oct 11, 2024

Good catch!
/lgtm
/approve

Copy link
Contributor

openshift-ci bot commented Oct 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: atheo89

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jstourac
Copy link
Member Author

/override ci/prow/images

Thanks for your reivew, guys!

Copy link
Contributor

openshift-ci bot commented Oct 11, 2024

@jstourac: Overrode contexts on behalf of jstourac: ci/prow/images

In response to this:

/override ci/prow/images

Thanks for your reivew, guys!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 7fb4961 into opendatahub-io:main Oct 11, 2024
7 checks passed
@harshad16
Copy link
Member

harshad16 commented Oct 11, 2024

Thanks for the changes.
@jstourac , the numpy needs to be 1.26:


please correct me if i m looking at wrong place.

@jstourac
Copy link
Member Author

Hmm, did I check the wrong image content then? I'll try to check on Monday again, cause its weird then.

@jstourac
Copy link
Member Author

Thanks for the changes. @jstourac , the numpy needs to be 1.26:

please correct me if i m looking at wrong place.

@harshad16 , I checked the tensorflow image referenced in our params.env file currently. You can see that there are two Numpy versions shown by the quay.io actually:

numpy	1.26.4
numpy	2.1.1

Each was introduced in a different layer during the image build.

In the result image, we can truly see the following:

(app-root) sh-5.1# pip list | grep numpy
numpy                        1.26.4

Truth is that I checked this with the downstream for 2.14RC1 build only via the test. Since the other images have the numpy updated to 2.1 already and the test showed this for this TensorFlow image also, I simply wen't on and updated this. But same data that I checked with the upstream image, can be seen also for the downstream 2.14 RC1 build TensorFlow image.

So, I'll raise a new PR to revert this version and will check what the heck was with the test to understand this a bit.

Thank you for noticing!

@jstourac jstourac deleted the fixTensorflowCudaBundle branch October 13, 2024 13:44
jstourac added a commit to jstourac/notebooks that referenced this pull request Oct 13, 2024
This partially reverts the opendatahub-io#742 to fix the
expected version in the final image.
@jstourac
Copy link
Member Author

So, I understand why the test failed now. I misinterpreted the test failure - it actually failed on the reason that the Numpy package version doesn't match to what we have in the other packages. So I amended test so it doesn't expect Numpy (and couple more because of the recently added check for the Trusty AI image) to have the version aligned - red-hat-data-services/ods-ci#1923.

For the manifest partial revert of this PR, I raised #743 to make this consistent again.

@harshad16 thank you, again, for noticing this! 🙂

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

Successfully merging this pull request may close these issues.

4 participants