New torch instructions will make it harder for downstream packages to depend on ivadomed's Python API
#861
Comments
joshuacwnewton
added
the
dependencies
joshuacwnewton
changed the title
torch instructions will make it harder for downstream packages to depend on ivadomedtorch instructions will make it harder for downstream packages to depend on ivadomed's Python API
|
Hi from AxonDeepSeg 👋
I was been debugging for like an hour right now my ADS installation using IVADOMED as a (new) requirements, wondering why it was throwing me a missing torch package error when I was loading IVADOMED in python. I'd assume you guys had included it in your setup, if it was a requirement to run IVADOMED. So... yeah, +1 for this issue getting resolved. I saw no instructions before I had the idea to search "requirements" in your issues and found this. |
|
(I know I was being a bad user by not reading your ReadTheDocs thoroughly, sorry about that. But... not every user will 🤷) |
ALTHOUGH...... in my defence..... I just noticed that your ReadTheDocs button links to your stable release: https://ivadomed.org/en/stable/?badge=stable ...which is missing the instructions to install torch that you linked in this issue, which is from the latest release: https://ivadomed.org/en/latest/installation.html#approach-2-venv ...so it's not really my fault I missed it 🙃 I'll open an issue for that one. |
|
I think maybe we can try to implement this: https://discuss.pytorch.org/t/how-to-add-pytorch-as-a-requirement-in-setup-py/106188, namely a parsing function for setup.py so we can streamline the |
Using --extra-index-url instead of --find-links means there's no need to pin a torch version -- which means we can trust ivadomed to specify the version it needs instead. Fixes(?) ivadomed/ivadomed#861 (comment) Related: spinalcordtoolbox/spinalcordtoolbox#3790
* Rehome the entire installation process in setup.py, including refactoring `requirements_dev.txt` into `extras_require`. * Re-add `torch` and `torchvision` as formal dependencies. They were specified in requirements.txt files, but pip was never made aware. * Stop specifying particular `torch` and `torchvision` builds; default to using the default builds from PyPI like everyone else. The default builds are stable and widely supported, but have the downsides that Windows users can't use their GPUs while Linux users who don't have GPUs have to wait for a 700MB of download they will never use. But the downsides can be overridden with the repos at download.pytorch.org: `pip install --extra-index-url https://download.pytorch.org/whl/cpu ivadomed` gets the smaller CPU-only build -- useful for Linux users without GPUs. `pip install --extra-index-url https://download.pytorch.org/whl/cu102 ivadomed` gets the CUDA 10.2 builds -- useful for those Windows users. `pip install --extra-index-url https://download.pytorch.org/whl/cu111 ivadomed` gets CUDA 11.1 builds -- useful for users with GPUs that are so new CUDA 10 is incompatible. By removing direct mentions of download.pytorch.org from ivadomed this lets most people use the well tested builds torch publishes, while leaving the way open for downstream projects or sysadmins to adapt ivadomed to their own systems. For example, this should lessen the headaches we've had with ComputeCanada, by transparently using their in-house torch builds. * CUDA 11 is no longer mandatory -- it never really was. * As a lucky side-effect, switching GPU users to CUDA 10 gives a speed boost: * axondeepseg/axondeepseg#642 (comment) * pytorch/pytorch#51044 * `torch` is loosened to `torch~=1.8.1` (meaning 1.8.1, 1.8.2, 1.8.3, …), which makes it easier for `--extra-index-url` to find a matching build. Fixes #861 by re-adding `torch`/`torchvision`. Fixes #1130 which is more or less a duplicate. Resolves #996 by sidestepping the problem. Closes #1000 by superseding it. To be followed up by matching docs changes in #1125. Co-authored-by: Kanishk <36276423+kanishk16@users.noreply.github.com>
ivadomed now <ivadomed/ivadomed#1129> specifies torch as a standard dependency, without giving special manual instructions for it. This means there's finally no need to pin a torch version here in axondeepseg; previously, we had to match their version in our environment.yml because it was a manual step in their install instructions. But now the metadata makes pip handle it. ivadomed gave special install instructions to try to handle the complicated mess that it hardware (i.e. CPU vs GPU vs hyper modern CUDA-11-only GPUs) compatibility. But now they're just leaning on the 80%-20% rule of thumb: >80% of people are covered by the default torch builds. And we know how to handle the <20% cases of people with CUDA-11-only GPUs or Windows machines with GPUs by having them install with '--extra-index-url = https://download.pytorch.org/whl/<hardware-specific-build>' axondeepseg was pushing the CPU version of torch on people, assuming that most users would not own GPUs and that in the Linux case it would be very wasteful, maybe even make ADS uninstallable for some users. We gave our own manual instructions that duplicated ivadomed's for how to switch to the GPU version. That process meant double the downloads and likely mismatched versions, and maintenance work on our end as we would have to update the manual instructions whenever ivadomed did. Instead, follow torch's defaults, like ivadomed now does. But since the users with Linux and CPU-only hardware is still fairly common, highlight --extra-index-url as an optional *pre*step, allowing for the same final install but avoiding the double-download and likely versioning mistakes. Overall: - Windows and macOS continue to have no GPU support - Linux always has GPU support, even if it doesn't need it; this balloons the default install, but anyone using torch now has this same problem and the consensus seems to be just to pay for bigger disks that can tolerate the waste - The relatively common case of Linux users without GPUs have an escape hatch to avoid the large download - Windows users with GPUs have a way to use them (`--extra-index-url https://download.pytorch.org/whl/cu102`), but we don't document nor support this - Linux/Windows users with super new CUDA-11-only GPUs also have a way to use them, which we we also don't document nor support directly Fixes ivadomed/ivadomed#861 (comment) Related: spinalcordtoolbox/spinalcordtoolbox#3790 In passing, remove the redundant parts of the `conda env create` line. Co-authored-by: Mathieu Boudreau, PhD <emb6150@gmail.com> Co-authored-by: Armand <83031821+hermancollin@users.noreply.github.com>
In #757,
ivadomedstopped listingtorch+torchvisionas default dependencies, instead opting to ask users to choose between the GPU/CPU versions and install them themselves. But, I think this change might make it harder for any downstream packages to depend onivadomed:ivadomedin their package requirements (e.g. viasetup.py->install_requires=), thenivadomedwill be broken out of the box (sincetorchis missing).setup.pycan't clone a repository to installtorchviarequirements_gpu.txt/requirements.txt).torchas one of their dependencies on behalf ofivadomed.spinalcordtoolboxis currently doing, which is easy enough for us, because SCT's development has been somewhat intertwined with ivadomed's development.The text was updated successfully, but these errors were encountered: