Add package contraints to torchbench #2318
Conversation
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
|
@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
|
@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
utils/python_utils.py
Outdated
| import sys | ||
| constraints_file = REPO_DIR.joinpath("build", "constraints.txt") | ||
| if not constraints_file.exists(): | ||
| warnings.warn("constraints.txt could not be found, please rerun install.py.") |
There was a problem hiding this comment.
My read here is that build/constraints.txt is an optional constraints file that TorchBench users could provide. But the warning here seems to says that the file is a requirement?
There was a problem hiding this comment.
Yes, it is an optional constraint. We could update the warning message to sth like:
The build/constrants.txt file is not found. Please consider rerunning the install.py script to generate it.
It is recommended to have the build/constrants.txt file to prevent unexpected version change of numpy or torch.
There was a problem hiding this comment.
Ah ok, I see the generate_pkg_constraints func above now
|
|
||
|
|
||
| def create_conda_env(pyver: str, name: str): | ||
| command = ["conda", "create", "-n", name, "-y", f"python={pyver}"] | ||
| subprocess.check_call(command) | ||
|
|
||
|
|
||
| def pip_install_requirements(requirements_txt="requirements.txt", |
There was a problem hiding this comment.
I'm not sure if it's easy to add a verbose option here and when it's set to true, print out the output of the pip install command below. When I used install.py locally, it installs stuffs silently even with -v, which makes it a bit slow to figure out which models install what
There was a problem hiding this comment.
Adding a verbose option sounds reasonable, but it will be non-trival to turn it on across all models, since each model has its own install.py.
|
@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
xuzhao9
had a problem deploying
to
docker-s3-upload
GitHub Actions
Failure
|
@xuzhao9 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
This PR introduces package constraints because we want to guarantee that certain packages are the same version before and after the installation. This is important for the CI in pytorch/pytorch because overriding critical package versions will break the CI due to ABI incompatibility.
If we find a certain model breaks the constraints in the future, we could ask the model installer to apply the
constraint.txtfile with their requirements.txt file.PyTorch CI Test: pytorch/pytorch#129114