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 ordering of tf and tflite installs in ci_cpu #8312

Merged
merged 1 commit into from
Jun 24, 2021

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Jun 23, 2021

The recently merged #8306 introduced a depedencyfor tflite installation that tf must be installed first.However, that PR did not correct the ordering in ci_cpu which does not have that ordering.

cc : @leandron @mbrookhart @Lunderberg

@Lunderberg
Copy link
Contributor

Thank you for catching that. It looks like the Dockerfile.ci_qemu will run into the same issue.

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.

Thank you

Copy link
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Thanks @manupa-arm!

Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 23, 2021
Similar to apache#8312, the recently merged apache#8306 required tflite to be
installed after tf.  However, apache#8306 did not correct the ordering in
the dockerfiles.  After this and apache#8312, all dockerfiles should be up
to date with the correct ordering.
@leandron
Copy link
Contributor

leandron commented Jun 23, 2021

CI failed on this one, due to a timeout. But CI doesn't really test anything related to the shell scripts on this PR. Based on that I'm gonna merge it, as I understand this was tested locally and is a necessary change. edit: we'll actually I can't.

@manupa-arm can you poke CI once you have a moment, so that we can merge this?

The recently merged 8306 PR introduced a depedency
for tflite installation that tf must be installed first.
However, that PR did not correct the ordering in ci_cpu which
does not have that ordering.

Change-Id: Ib82c2b33e4e123d4562682e9e97b21bfe23cc0ef
@manupak manupak force-pushed the fix_tf_tflite_order_ci_cpu branch from 7ca8373 to 174ec44 Compare June 23, 2021 19:25
@manupak
Copy link
Contributor Author

manupak commented Jun 23, 2021

Alright -- re-triggered it

Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 23, 2021
Similar to apache#8312, the recently merged apache#8306 required tflite to be
installed after tf.  However, apache#8306 did not correct the ordering in
the dockerfiles.  After this and apache#8312, all dockerfiles should be up
to date with the correct ordering.
@leandron leandron merged commit 3db44f4 into apache:main Jun 24, 2021
Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Jun 24, 2021
Similar to apache#8312, the recently merged apache#8306 required tflite to be
installed after tf.  However, apache#8306 did not correct the ordering in
the dockerfiles.  After this and apache#8312, all dockerfiles should be up
to date with the correct ordering.
mbrookhart pushed a commit that referenced this pull request Jun 25, 2021
Similar to #8312, the recently merged #8306 required tflite to be
installed after tf.  However, #8306 did not correct the ordering in
the dockerfiles.  After this and #8312, all dockerfiles should be up
to date with the correct ordering.

Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
The recently merged 8306 PR introduced a depedency
for tflite installation that tf must be installed first.
However, that PR did not correct the ordering in ci_cpu which
does not have that ordering.

Change-Id: Ib82c2b33e4e123d4562682e9e97b21bfe23cc0ef
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
Similar to apache#8312, the recently merged apache#8306 required tflite to be
installed after tf.  However, apache#8306 did not correct the ordering in
the dockerfiles.  After this and apache#8312, all dockerfiles should be up
to date with the correct ordering.

Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
zxy844288792 pushed a commit to zxy844288792/tvm that referenced this pull request Mar 4, 2022
The recently merged 8306 PR introduced a depedency
for tflite installation that tf must be installed first.
However, that PR did not correct the ordering in ci_cpu which
does not have that ordering.

Change-Id: Ib82c2b33e4e123d4562682e9e97b21bfe23cc0ef
zxy844288792 pushed a commit to zxy844288792/tvm that referenced this pull request Mar 4, 2022
Similar to apache#8312, the recently merged apache#8306 required tflite to be
installed after tf.  However, apache#8306 did not correct the ordering in
the dockerfiles.  After this and apache#8312, all dockerfiles should be up
to date with the correct ordering.

Co-authored-by: Eric Lunderberg <elunderberg@octoml.ai>
Sign up for free to 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