Skip to content

Upgrade tensorflow to version to 2.6.x - #10084

Merged
leandron merged 1 commit into
apache:mainfrom
ophirfrish:tvmc-upgrade-tensorflow
Mar 17, 2022
Merged

Upgrade tensorflow to version to 2.6.x#10084
leandron merged 1 commit into
apache:mainfrom
ophirfrish:tvmc-upgrade-tensorflow

Conversation

@ophirfrish

@ophirfrishophirfrish commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

Upgrade the following versions:
keras - from 2.4.3 to 2.6
tensorflow - from 2.4.2 to 2.6.2
h5py - from version < 3.0 to version 3.1.0

cc @areusch@leandron@Mousius@gromero for reviews

Co-authored-by: Chris Sidebottom Chris.Sidebottom@arm.com

@lhutton1lhutton1 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.

Thanks @ophirfrish! Just a small thing below

Comment threaddocker/install/ubuntu_install_tensorflow.sh
@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch from 795bcf7 to 1c775e3CompareJanuary 31, 2022 14:20

@lhutton1lhutton1 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.

LGTM!

@leandronleandron self-assigned this Feb 1, 2022
@leandron

leandron commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

I'm using this PR to generate a set of temporary Docker images so that we can validate the proposed change: https://ci.tlcpack.ai/blue/organizations/jenkins/docker-images-ci%2Fdaily-docker-image-rebuild/detail/daily-docker-image-rebuild/204/pipeline

@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch 2 times, most recently from 20f4dcb to ab5acb0CompareFebruary 2, 2022 05:23
@leandron

Copy link
Copy Markdown
Contributor

@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch from ab5acb0 to b94e252CompareFebruary 16, 2022 10:29
@leandron

Copy link
Copy Markdown
Contributor

@jtuyls@anilmartha, in the process of updating TF to 2.6 we (@ophirfrish@Mousius and me) bumped into an issue that cpptests were crashing, which turns out to be some conflicting h5py (versions 2.x and 3.x) installations in some Vitis related scripts. We are proposing to unify the h5py installation to be done only in the TF script.

Can you please review and in case you disagree, propose some options to those changes?

@jtuyls

Copy link
Copy Markdown
Contributor

@jtuyls@anilmartha, in the process of updating TF to 2.6 we (@ophirfrish@Mousius and me) bumped into an issue that cpptests were crashing, which turns out to be some conflicting h5py (versions 2.x and 3.x) installations in some Vitis related scripts. We are proposing to unify the h5py installation to be done only in the TF script.

Can you please review and in case you disagree, propose some options to those changes?

@leandron Could we keep the h5py being installed separately in the vitis_ai_core.sh script: https://github.com/apache/tvm/pull/10084/files#diff-7773869aa5e34502bbaa476fe3339c2e3956199b0be759084feee12fe8bcdfe1L37?
In our Dockerfile we don't use the ubuntu_install_tensorflow.sh script so h5py won't be installed otherwise. This shouldn't affect the CI as this script is not used there.
The changes in vitis_ai_packages_ci script are fine for us.

@leandron

leandron commented Feb 16, 2022

Copy link
Copy Markdown
Contributor

@leandron Could we keep the h5py being installed separately in the vitis_ai_core.sh ?

Sure, and with regards to versions, do you want to keep it as is, with version 2.x or update to be coherent with the one installed in TVM, which means h5py 3.x ?

@jtuyls

Copy link
Copy Markdown
Contributor

@leandron Could we keep the h5py being installed separately in the vitis_ai_core.sh ?

Sure, and with regards to versions, do you want to keep it as is, with version 2.x or update to be coherent with the one installed in TVM, which means h5py 3.x ?

Yeah, I think it's best to keep it as is (2.10.0) for now and I can work on updating it later to 3.x. I think 3.x works but I would like to completely verify that first.

@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch 2 times, most recently from 71e17da to c48010bCompareFebruary 24, 2022 13:19
@leandron

Copy link
Copy Markdown
Contributor

@leandron

leandron commented Mar 2, 2022

Copy link
Copy Markdown
Contributor

With the latest version of this PR, rebased on top of main at bd14a4d36e0d364ef9bd34b2ee96cc09ce64d4b3, and with #10406 applied, I generated a new set of Docker images for testing at https://ci.tlcpack.ai/job/docker-images-ci/job/daily-docker-image-rebuild/238/, with label tlcpackstaging/ci_****:20220301-163838-977bdd3d2.

@leandron

Copy link
Copy Markdown
Contributor

This is validated in https://ci.tlcpack.ai/blue/organizations/jenkins/tvm/detail/ci-docker-staging/232/pipeline/, covering up to 4797850.

@ophirfrish can you trigger CI again, so that I can merge this?

@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch from c48010b to 2fef747CompareMarch 17, 2022 12:53
Upgrade the following versions:
keras - from 2.4.3 to 2.6
tensorflow - from 2.4.2 to 2.6.2
h5py - from version < 3.0 to version 3.1.0
@ophirfrish
ophirfrishforce-pushed the tvmc-upgrade-tensorflow branch from 2fef747 to ea460f0CompareMarch 17, 2022 13:13

@leandronleandron 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.

LGTM, thanks @ophirfrish!

@leandron
leandron merged commit 1290213 into apache:mainMar 17, 2022
@leandron

leandron commented Mar 17, 2022

Copy link
Copy Markdown
Contributor

This is merged now, as it is running in production CI starting on tlcpack/ci-gpu:v0.82 and tlcpack/ci-cpu:v0.82 in #10654.

Thanks @ophirfrish@lhutton1 and @jtuyls!

pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
Upgrade the following versions:
keras - from 2.4.3 to 2.6
tensorflow - from 2.4.2 to 2.6.2
h5py - from version < 3.0 to version 3.1.0
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ophirfrish@leandron@jtuyls@lhutton1