sure, python/requirements/requirements.txt?
how about python/requirements.txt to follow normal convention, and then python/extra-requirements/tflite.txt
Alternatively, we can move all the extra requirements into a single file, given that it is relatively tedious for users to install separately anyway. They can cherry pick if they want
I’m starting to work on this one. One small departure we need to make from the previously-agreed-upon plan (from a few replies back): as part of this plan, we’re defining one extras_require for each TVM frontend. Some frontends depend on the same external package, but we need to ensure that where we place semver constraints on such packages, we do it uniformly across TVM.
Therefore, I propose we generate the set of requirements-*.txt files from a small Python tool python/gen_requirements.py. this tool would contain a specification for requirements like:
requirements_by_extra = {
"tvm": ["attrs", "decorator", ...], # Core requirements
"importer-tensorflow": ["tensorflow", "tensorflow-estimator"],
"importer-tflite": ["tflite", "tensorflow", "tensorflow-estimator"],
}
constraints = {
"tensorflow": "^2.1", # Maps package name to semver or pip-style requirements
}
we could then either:
R1. Check in the requirements-*.txt, and create a lint step to verify that the checked-in, generated requirements-*.txt matches the Python spec in gen_requirements.py
R2. .gitignore the generated requirements-*.txt, and invoke gen_requirements.py from setup.py.
I sort of lean towards R2, but open to suggestions.
Hi @areusch, I’m sorry I’m a bit late, but was reading through the whole thread and got curious, as we are talking about Python dependencies, on what we envisage as the experience for an end-user, installing a TVM package generated using the proposed mechanisms.
At the moment, if we generate a TVM package (purely based on apache/tvm repo) and install it on a new Virtualenv, we don’t really get something that we can use to import a model, as none of the frontend dependencies will be in place.
It is also not very usual for users being expected to consume “requirements.txt”-style files directly. What usually is there, is a clean pip install <something>, that will install all dependencies required.
Q1: Is that correct to assume the proposal is that the user will be expected to pip install tvm[importer-tensorflow] (I’m using “tvm” as package here, just as a placeholder), which will install tvm with all required dependencies for TensorFlow frontend enabled, but in case the same user now needs to use ONNX or PyTorch, it will be broken until the user manually install the dependency?
Q2: I know a lot of thinking and effort was put into the current mechanism being designed, but I’m curious on why don’t we just declare all the dependent frameworks and versions, and let the package to install everything, rather than we needing to keep an elaborate mechanism of dependencies processing and files being generated? It seems to me that it would make the experience much more aligned with everything else in the ecosystem.
hey @leandron, thanks for chiming in! here are some answers.
It is also not very usual for users being expected to consume “
requirements.txt”-style files directly.
definitely. requirements.txt is purely intended for use with the tvm source repo. there are a lot of different virtualenv management tools, but most (all?) of them can consume requirements.txt. theoretically, it should also be possible to eventually install tvm in “editable” mode, but currently doing so requires that you’ve built libtvm.so, so using requirements.txt makes it a more useful “official” dependencies list, for source tree purposes.
Q1: Is that correct to assume the proposal is that the user will be expected to
pip install tvm[importer-tensorflow]( I’m using “tvm” as package here, just as a placeholder ), which will install tvm with all required dependencies for TensorFlow frontend enabled, but in case the same user now needs to use ONNX or PyTorch, it will be broken until the user manually install the dependency?
technically there’s a packaging flow that could further modify setup() arguments such that the end user flow could do something different or enable certain importers by default. but absent that (and I don’t believe the flow does anything like that now), that seems correct.
Q2: I know a lot of thinking and effort was put into the current mechanism being designed, but I’m curious on why don’t we just declare all the dependent frameworks and versions, and let the package to install everything, rather than we needing to keep an elaborate mechanism of dependencies processing and files being generated? It seems to me that it would make the experience much more aligned with everything else in the ecosystem.
I agree pip install foo is typically expected to be a batteries-included sort of command, but I think there are more practical reasons why installing all importer dependencies as default may be hard right now. for example, PyTorch is nearly 800MB. I think @tqchen may have more examples, but at least from a source code perspective, I believe the paradigm we’re attempting to embrace is to minimize the set of required dependencies to make it possible to use TVM in a wide range of settings. it should be quite easy to fix up the TVM error message to explain to users "please run pip install tvm[importer-onnx]" if tvm finds a missing dependency.
I think the end-user flow is definitely worth debating. the initial goal of this proposal is merely to consolidate the set of python dependencies scattered around the source tree, so that’s what i care about. happy to make changes as the community’s thinking evolves around the end-user flow.
in implementing the draft PR and collecting the various package constraints from the codebase, i’ve now come across several different cases where we may want to add version constraints that aren’t strictly necessary for functional reasons. It’s not clear that these constraints should live in the gen_requirements.py tool from the PR. in scattering various pip3 install commands around the codebase, we’ve implicitly added this context, but we haven’t always documented why we’re restricting the version or the context for the restriction.
here are some contexts:
-
Functional Constraints – in some cases e.g.
synr,xgboost, we must constrain TVM to a particular min version to avoid bugs. as we check these in tomain, these will always be min version constraints and never max version constraints (an example of a max version constraint would be if we were making a release and discovered e.g. a just-released pytorch is incompatible and we have no time to patch–in this case, we want a release constraint, see below). -
Dev Constraints – some packages e.g.
pylint,cpplint,blackare purely used as developer tools. for best developer experience, their output should be the same across all installations. therefore, those packages and their dependencies should be pinned exactly to whatever version is used inci-lint. however, it should be possible to easily rebuildci-lintto use the latest version. -
Tested Constraints – people using TVM from the source tree should be able to install a set of dependencies that are the same version as is tested in the CI. However, these aren’t
ci-constraints.txtas mentioned in this proposal–we don’t want to constrain future CI container builds, we just want to document and use whatever was selected at the previous CI container build. -
Release Constraints – when building a tlcpack pip package, we may want to decide which of these constraints to impose on the generated wheels. for example, we may want to produce a pip package with no version constraints and one with CI-tested versions.
i’d propose to move forward with the current PR adding the Functional Constraints only to gen_requirements.py. However, the current PR doesn’t accommodate the remaining use scenarios and I don’t think we’ve fully addressed them here. Before we can address the remaining contexts, we’ll need to consider how all of this work is reflected in the CI–so, I’d like to merge this PR as an improvement to setup.py, and then work towards a follow-on RFC/PR which proposes changes to the process by which we build and update CI containers.
My two cents:
For the Dev constraints, if we don’t find any mismatch outputs, it would be more flexible to have a range of versions instead of just a single one; otherwise it might be tedious for many people that work on many projects at the same time, and each project may have no intersect to the pylint version, for example.
Meanwhile, I agree with you that the Tested constraints used in CI should be an exact version. IIUC, currently those constraints are specified in the docker files and the installation scripts, but not as strict as you proposed, of course. Maybe we can just simply make them comprehensive there?
Other than that, it makes sense to have the current PR first covering Functional constraints only to keep it concise and clean.
@comaniac I’d propose that we don’t include the dev constraints in setup.py (including in any future RFC/PR). This should let you pip install -r python/requirements/dev.txt without constraining any versions (note the PR does not look like this right now).
I have noticed with Python projects is that pylint/astroid in particular are extremely sensitive to the version and even the cpython version being used. I agree a range of dependencies can sometimes make sense, but for exactly reproducing the CI output locally, I think exact pinning is needed.
Then again, there is another debate to be had as to “what is the process for exactly reproducing the CI?” Arguably, docker/bash.sh tlcpack/ci-lint:ver is the way to reproduce the CI, and one shouldn’t have high expectations for other procedures. However, you do currently have to download several docker containers (and you actually must have a GPU and nvidia-docker present locally) even to just try installing dev dependencies with exact versions into a Python venv, and that seems unnecessarily burdensome.
This should be discussed more in a follow-on RFC, but as a preview, my thinking is that when a Docker container version is bumped in Jenkinsfile, a separate e.g. ci-lint-freeze.txt should be updated somewhere which records the output of pip freeze from the container. Then, tools can post-process that to convert the precise pins from pip freeze into looser constraints according to a package’s versioning policy (e.g. for semver packages, convert ==x.y.z into ^x.y.z or ~x.y.z). These tools can drive Dev Constraints, CI-tested Constraints, or Release Constraints.
Meanwhile, I agree with you that the Tested constraints used in CI should be an exact version. IIUC, currently those constraints are specified in the docker files and the installation scripts, but not as strict as you proposed, of course. Maybe we can just simply make them comprehensive there?
Some discussion is also needed as to how CI containers actually pip install packages and whether some type of poetry-style locking should be used. And, some discussion is needed around intentionally CI-testing a range of packages to improve our test coverage. Presumably, some separate process is needed to generate a container-wide constraints.txt, but I believe there’s already some contention as to how much complexity we should incorporate into our container build process. So, let’s discuss this all in the follow-on RFC.
1 Like