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.

Sign up for GitHub

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

Jump to bottom

[TFLite][CI] Update TensorFlow dependency to 2.9.1 #12131

Merged
merged 1 commit into from
Sep 7, 2022
Merged

[TFLite][CI] Update TensorFlow dependency to 2.9.1 #12131

merged 1 commit into from
Sep 7, 2022

Conversation

leandron
Copy link
Contributor

@leandron leandron commented Jul 19, 2022 •
edited by github-actions bot
Loading

This updates the TF version to be used in TVM CI to 2.9.1, which brings improvements so that more platforms are supported by official packages. This PR updates the Docker images scripting to install TF and TFLite and some CMake changes required when updating from TF 2.6.x to 2.9.x.

Adding it here as a draft until we get #12130 merged.

cc @Mousius @NicolaLancellotti @areusch @driazati @gigiblender

@leandron leandron force-pushed the tflite_29_update branch 3 times, most recently from 8f0fa97 to 2dd5353 Compare July 19, 2022 16:46
@areusch
Copy link
Contributor

areusch commented Jul 25, 2022

@leandron when investigating the Python deps I also came across this bug. We currently build a tflite library from the tensorflow source, and in TF 2.7 the build system was switched to CMake I believe. I haven't yet had time to solve this, but I'll let you know if I do.

@leandron
Copy link
Contributor Author

@leandron when investigating the Python deps I also came across this bug. We currently build a tflite library from the tensorflow source, and in TF 2.7 the build system was switched to CMake I believe. I haven't yet had time to solve this, but I'll let you know if I do.

This is solved in this PR. The only issue is that the CI is testing with the old images. I'll generate a new set from ci-docker-staging and re-run the tests. it should be fine.

@areusch
Copy link
Contributor

areusch commented Jul 25, 2022

hm, was that https://ci.tlcpack.ai/blue/organizations/jenkins/docker-images-ci%2Fdocker-image-run-tests/detail/docker-image-run-tests/171/pipeline?

@leandron
Copy link
Contributor Author

Yes. However it still requires the Docker images to have the updated TF for the whole CI.

I generated a new set, and will make a new CI staging run with the new images. Then it should be fine.

@areusch
Copy link
Contributor

areusch commented Jul 26, 2022

ah ok cool--lmk when i can take a look at them

@NicolaLancellotti NicolaLancellotti mentioned this pull request Jul 26, 2022
Closed
@leandron leandron mentioned this pull request Aug 23, 2022
Merged
@leandron leandron mentioned this pull request Aug 31, 2022
Merged
@leandron leandron marked this pull request as ready for review September 2, 2022 15:32
@leandron
Copy link
Contributor Author

leandron commented Sep 2, 2022 •
edited
Loading

@areusch just to update here as we're discussing, once this is merged, the PR that updates the Docker images, will need to update task_config_build_cpu.sh from:

echo set\(USE_TFLITE ON\) >> config.cmake

to:

echo set\(USE_TFLITE /opt/tflite\) >> config.cmake

This way we keep the default source based installation in the new format introduced here.

If I happen to do the update myself, I'll do that update above, but it will be evident if this is not there, the build will fail at Docker image testing time.

@areusch
Copy link
Contributor

areusch commented Sep 2, 2022

@mbrookhart could you have a look?

mbrookhart
mbrookhart reviewed Sep 2, 2022
View reviewed changes
Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

A couple of minor comments, but overall I think I'm okay with this.

docker/Dockerfile.ci_cpu Outdated Show resolved Hide resolved
docker/install/ubuntu_install_cmake_source.sh Outdated Show resolved Hide resolved
@leandron leandron force-pushed the tflite_29_update branch 2 times, most recently from af4871f to d56e23f Compare September 2, 2022 18:09
driazati
driazati approved these changes Sep 2, 2022
View reviewed changes
Copy link
Member

@driazati driazati left a comment

Choose a reason for hiding this comment

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

lgtm once comments are addressed

docker/install/ubuntu_install_cmake_source.sh Outdated Show resolved Hide resolved
@leandron
Copy link
Contributor Author

leandron commented Sep 2, 2022

lgtm once comments are addressed

Thanks @driazati

@leandron
Copy link
Contributor Author

leandron commented Sep 2, 2022

@tvm-bot rerun

@leandron
Copy link
Contributor Author

leandron commented Sep 5, 2022

@tvm-bot rerun

@leandron leandron force-pushed the tflite_29_update branch 2 times, most recently from 22fd81d to 4bc5e11 Compare September 5, 2022 10:39
@leandron
[TFLite][CI] Update TensorFlow dependency to 2.9.1
73c0dea 
This updates the TF version to be used in TVM CI to 2.9.1,
which brings improvements so that more platforms are supported by
official packages.

When building TFLite, an update to CMake was also required,
which is updated now to 3.18.4.

ethos-u-vela dependency is also updated, from version 3.2.0 to 3.4.0
so that it is closer to the TensorFlow version being proposed here.

This PR updates the Docker images scripting to install TF and TFLite.

Change-Id: I290085f0c018ad57606f1295494c19ff6e1af2dd
@mehrdadh
Copy link
Member

mehrdadh commented Sep 7, 2022 •
edited
Loading

Looks good to me, just wanted to mention that this might break microTVM testing on hardware since RVM uses a different tensorflow version. If it does we need a follow up PR.

tvm/apps/microtvm/pyproject.toml

Line 107 in 2622ac9

tensorflow = {version = "^2.1", optional = true}

@leandron
Copy link
Contributor Author

leandron commented Sep 7, 2022

Looks good to me, just wanted to mention that this might break microTVM testing on hardware since RVM uses a different tensorflow version. If it does we need a follow up PR.

tvm/apps/microtvm/pyproject.toml

Line 107 in 2622ac9

tensorflow = {version = "^2.1", optional = true}

Did this change recently? I’ve run many test rounds and haven’t seen any breakage in microtvm

@areusch areusch merged commit bee5627 into apache:main Sep 7, 2022
@areusch
Copy link
Contributor

areusch commented Sep 7, 2022

thanks @leandron !!

@mehrdadh
Copy link
Member

mehrdadh commented Sep 7, 2022

@leandron I don't think it changed recently, it should be fine

@driazati driazati mentioned this pull request Sep 7, 2022
Merged
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
@leandron @xinetzone
[TFLite][CI] Update TensorFlow dependency to 2.9.1 (apache#12131)
6c7da2e 
This updates the TF version to be used in TVM CI to 2.9.1,
which brings improvements so that more platforms are supported by
official packages.

When building TFLite, an update to CMake was also required,
which is updated now to 3.18.4.

ethos-u-vela dependency is also updated, from version 3.2.0 to 3.4.0
so that it is closer to the TensorFlow version being proposed here.

This PR updates the Docker images scripting to install TF and TFLite.

Change-Id: I290085f0c018ad57606f1295494c19ff6e1af2dd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookhart mbrookhart mbrookhart left review comments

@driazati driazati driazati approved these changes

@Mousius Mousius Awaiting requested review from Mousius

@areusch areusch Awaiting requested review from areusch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@leandron @areusch @mehrdadh @mbrookhart @driazati