You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/03/26 13:50:10 UTC
[GitHub] [tvm] leandron opened a new issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
leandron opened a new issue #7751:
URL: https://github.com/apache/tvm/issues/7751
There are some dependencies that are inconsistently declared within Docker setup scripts and some CI test scripts. The cases I noticed are declared within `docker/install/ubuntu_install_python_package.sh`, `tests/scripts/task_ci_python_setup.sh` and `tests/scripts/task_ci_setup.sh` with different version constraints:
1. `tlcpack-sphinx-addon` is missing from the Docker setup
2. `synr` version specification is inconsistent
https://github.com/apache/tvm/blob/f4490708a559930c6554eec26c377d49bfb46d84/docker/install/ubuntu_install_python_package.sh#L23-L24
https://github.com/apache/tvm/blob/f4490708a559930c6554eec26c377d49bfb46d84/tests/scripts/task_ci_python_setup.sh#L32-L33
https://github.com/apache/tvm/blob/f4490708a559930c6554eec26c377d49bfb46d84/tests/scripts/task_ci_setup.sh#L32-L33
I'm opening this as an issue, and not a PR directly, because I wanted to see how we can improve this, in line with https://discuss.tvm.apache.org/t/rfc-python-dependencies-in-tvm-ci-containers/9011.
cc @tqchen @areusch @mousius @u99127
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #7751:
URL: https://github.com/apache/tvm/issues/7751#issuecomment-808237112
Thanks @leandron Because these two deps can be updated more frequently(and ci image update cycle is slower), we choose to use ci_setup script to install them instead of hard baking the CI image. We should also remove synr from ci the docker image for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leandron commented on issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
Posted by GitBox <gi...@apache.org>.
leandron commented on issue #7751:
URL: https://github.com/apache/tvm/issues/7751#issuecomment-808327665
> yeah I also hit this problem. I think we need to fix the CI image update process so it is not such a burden. My opinion is we should implement one method to include dependencies in TVM CI and support that method, and avoid making CI-time additions like these two. I hit this problem with updating ci-arm and ci-cpu containers as well.
> I think if we make the CI image update process less of a burden, it wouldn't be a big deal to ask us to update all ci-* containers at once, rather than having ci-cpu built 3 weeks ago and ci-gpu built 3 months ago.
I think updating all images at once would be really good, in terms of keeping track of changes that happen in our dependencies.
Specifically in this case, even if we rebuild (**not** from scratch) the image, we'll still end up with the old `synr` version, because Docker will use the cached layer, as the hash of script that install those dependencies didn't change, hence the need for the script to perform the ad-hoc dependency installation.
So, I think [your proposal](https://discuss.tvm.apache.org/t/rfc-python-dependencies-in-tvm-ci-containers/9011) comes in very handy. When any python dependency version change, that consolidated file hash will change, and trigger a proper rebuild of the layers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #7751:
URL: https://github.com/apache/tvm/issues/7751#issuecomment-808358438
@areusch they are pinned because we want to make sure no cache is needed(e.g. if we change version there will be automatic update process, but won't reinstall other wise).
Note that in the best case scenario we still need a two step in order to update the image(binary image change then the code change) so having a setup hook for fast changing deps might be handy so the developers can directly see the effect of their change. I agree though that if the process improves or the deps stablizes(both will happen) we will get to the other land where docker should be the only dep manager
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
Posted by GitBox <gi...@apache.org>.
areusch commented on issue #7751:
URL: https://github.com/apache/tvm/issues/7751#issuecomment-808314918
yeah I also hit this problem. I think we need to fix the CI image update process so it is not such a burden. My opinion is we should implement one method to include dependencies in TVM and support that method, and avoid making CI-time additions like these two. I hit this problem with updating ci-arm and ci-cpu containers as well.
I think if we make the CI image update process less of a burden, it wouldn't be a big deal to ask us to update all ci-* containers at once, rather than having ci-cpu built 3 weeks ago and ci-gpu built 3 months ago.
What's not clear to me in this particular case is why synr and tlcpack-sphinx-addon are pinned. @tqchen do you have more insight into that--is it just to ensure the CI is predictable (I.e. because these packages are installed every CI run rather than each container build), or was there another reason?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch edited a comment on issue #7751: Inconsistent python dependencies "tlcpack-sphinx-addon, synr" in CI and Docker setup scripts
Posted by GitBox <gi...@apache.org>.
areusch edited a comment on issue #7751:
URL: https://github.com/apache/tvm/issues/7751#issuecomment-808314918
yeah I also hit this problem. I think we need to fix the CI image update process so it is not such a burden. My opinion is we should implement one method to include dependencies in TVM CI and support that method, and avoid making CI-time additions like these two. I hit this problem with updating ci-arm and ci-cpu containers as well.
I think if we make the CI image update process less of a burden, it wouldn't be a big deal to ask us to update all ci-* containers at once, rather than having ci-cpu built 3 weeks ago and ci-gpu built 3 months ago.
What's not clear to me in this particular case is why synr and tlcpack-sphinx-addon are pinned. @tqchen do you have more insight into that--is it just to ensure the CI is predictable (I.e. because these packages are installed every CI run rather than each container build), or was there another reason?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org