Add new scripts for building and cleaning wheels #925
Conversation
These encapsulate instructions from Michael Broughton about how to build new versions of TensorFlow Quantum.
The final print statement about the directory is misleading when `build_pip_package.sh` is being run inside a Docker container. On balance, I think it's better to remove it. Hopefully the user running the script will remember that they supplied the destination parameter as an argument on the command line to begin with.
mhucka
added
area/devops
Per Google guidance, executable scripts should use `/bin/bash`.
MichaelBroughton
left a comment
MichaelBroughton
left a comment
There was a problem hiding this comment.
Thanks for taking a stab at writing down the instructions. A few minor nits in the bash files themselves and then consider removing the python tests. After that LGTM.
| 1. Git clone the TensorFlow Quantum repo to a directory on your computer. | ||
|
|
||
| 1. `cd` into the local clone directory. | ||
|
|
||
| 1. Create a Python virtual environment. | ||
|
|
||
| 1. Run `pip install -r requirements.txt` | ||
|
|
||
| 1. Verify the major.minor version of Python you are using. The rest of these | ||
| instructions use 3.11 as an example. | ||
|
|
||
| 1. Run `./release/build_distribution.sh -p 3.11` | ||
|
|
||
| 1. If the above succeeds, it will leave the wheel in `/tmp/tensorflow_quantum/` | ||
| on your system. Take note of the name of the wheel file that | ||
| `build_distribution.sh` prints when it finishes. | ||
|
|
||
| 1. Run `./release/clean_distribution.sh /tmp/tensorflow_quantum/WHEEL_FILE`, | ||
| where `WHEEL_FILE` is the file noted in the previous step. If this works, it | ||
| will create a new wheel file in `../wheelhouse`. If an error occurs, it will | ||
| hopefully report the problem. If the error is a platform tag mismatch, run | ||
| `./release/clean_distribution.sh -s /tmp/tensorflow_quantum/WHEEL_FILE`; | ||
| this will run auditwheel's `show` command on the wheel file to indicate what | ||
| version of `manylinux` this wheel can be made to run on if you use | ||
| `auditwheel` to repair it. With that information, you may be able to edit | ||
| the `build_distribution.sh` script to experiment with different values for | ||
| the Crosstool and/or the Docker images used. | ||
|
|
||
| 1. Test the wheel. Go to a remotely hosted Colab (or any other Linux platform | ||
| that is distinctly difference from yours) upload this new generated wheel | ||
| file to local storage in the Colab, and test if it works. (E.g., try to run | ||
| `https://www.tensorflow.org/quantum/tutorials/hello_many_worlds` with the | ||
| new TensorFlow and TFQ wheels to verify things are working smoothly). | ||
|
|
||
| 1. Repeat the `build_distribution.sh` and `clean_distribution.sh` steps for | ||
| different versions of Python. |
There was a problem hiding this comment.
Nit: All steps have the same number.
There was a problem hiding this comment.
Yeah, it's a Markdown feature: you can avoid having to manually number (and renumber) lists with this approach. The list get proper numbered when rendered (see https://github.com/mhucka/quantum/blob/1377c8402b6c10994657a610c4cd8cddfdae0f2f/release/README.md#procedure). It's a maintainability win because moving stuff around doesn't require renumbering.
The Google markdown style guidelines say to use this approach for longer lists: https://google.github.io/styleguide/docguide/style.html#use-lazy-numbering-for-long-lists
| bazel build ${build_flags} release:build_pip_package | ||
| printf "\n:::::::: Creating Python wheel ::::::::\n\n" | ||
| bazel-bin/release/build_pip_package /build_output/ | ||
| if [ "${cleanup}" == "true" ]; then |
There was a problem hiding this comment.
Nit: prefer [[]] over []
There was a problem hiding this comment.
So, I wanted to use Bash constructs like that, but it wasn't clear whether there was a chance that /bin/bash would run in POSIX mode in some of those Docker containers. Avoiding the Bash constructs makes the script more portable.
However, this is probably overthinking it. We could assume Bash and just test if there's any problem. If it works in the containers now, it's unlikely the containers would ever go back to non-Bash in the future.
| rm -rf ${TMPDIR} | ||
| echo "$(date) : === Done." |
There was a problem hiding this comment.
Nit: why change this ?
There was a problem hiding this comment.
Yeah, I wrote a note in the commit message, but that text doesn't come through here.
What was happening here was this dumb situation: when this is run inside the docker container, it prints the path inside the container. To the person running the script, it is very confusing because instinctively you want to look into the directory it prints … and it doesn't actually exist on your local system. Since the person running the script will have typed the path as an argument (or can look back in their shell history to see what they typed), I reasoned that it wasn't a big loss to simply not print it and risk the confusion.
If there's a better solution to this, let me know!
| wheel_name="$(basename "${1}")" | ||
|
|
||
| args="" | ||
| if [ "${action}" = "repair" ]; then |
There was a problem hiding this comment.
Nit: use of = and [], prefer [[]] and ==
There was a problem hiding this comment.
Agreed. I'll convert it all to Bash syntax.
| "${docker_image}" \ | ||
| bash -c "auditwheel ${action} ${args} -w /tfq/wheelhouse /tmp/${wheel_name}" | ||
|
|
||
| if [ "${dry_run}" = "true" ]; then |
| @@ -0,0 +1,103 @@ | |||
| # Copyright 2025 Google LLC | |||
| # | |||
There was a problem hiding this comment.
I don't think we need python tests here. These bash scripts will likely change in subtle ways from version to version and having to update the tests as well as the scripts will likely end up being more work for whoever is doing it without all that much utility given from having them in the first place, but fine to leave if you feel strongly the other way.
There was a problem hiding this comment.
You're right; the maintenance costs outweigh the benefits. I'll remove them.
|
Thank you for your review. I'll update the PR after the release is done. |
2 participants
These new scripts in
release/encapsulate instructions from Michael Broughton about how to build new distributions of TensorFlow Quantum. The filerelease/README.mddescribes how to use them. There are unit tests as well; they can be invoked using bazel (and the process is also described in the README file).There are a couple of tweaks to
release/build_pip_package.shto take a debug tracing argument and make the script work inside the Docker containers run by the build script.(Note: this PR is meant to be merged after PR #913 is merged.)