Use Python 3.7 instead of Python 3.6 for SCT's conda environment

[joshuacwnewton]

Checklist

GitHub

• I've given this PR a concise, self-descriptive, and meaningful title
• I've linked relevant issues in the PR body
• I've applied the relevant labels to this PR
• I've applied a release milestone (major, minor, patch) in line with Semantic Versioning guidelines
• I've assigned a reviewer

PR contents

• I've consulted SCT's internal developer documentation to ensure my contribution is in line with any relevant design decisions
• I've added relevant tests for my contribution
• I've updated the relevant documentation for my changes, including argparse descriptions, docstrings, and ReadTheDocs tutorial pages

Description

This PR upgrades the Python version to 3.7. To make this happen, several other changes were needed:

1. tensorflow is updated to the latest 1.X version (1.15), and Keras is upgraded to match. (Also fixes 2 versions of tensorboard get installed with SCT #3035, meaning we no longer need the hacky tensorboard workaround.)
2. The copy-pasted argparse.Formatter methods are removed, and multiple-inheritance is used instead. (Fixes argparse._textwrap seems deprecated as of py3.7 #2782.)

Future work

• Upgrading tensorflow means that SCT is no longer available to users whose CPUs do not support the AVX instruction set. So, we would need consider to providing an alternative to those who need it. (See: Use Python 3.7 instead of Python 3.6 for SCT's conda environment #3361 (comment))
• Python 3.8 and Python 3.9 are still not supported.

Linked issues

Fixes #2782.
Fixes #3035.
Fixes #3227.
Fixes #3509.

Unblocks #3340.

[Drulex]

• Upgrading tensorflow means that SCT is no longer available to users whose CPUs do not support the AVX instruction set.

Just to add a correction here: the pre-built versions of tensorflow > v1.6 are using the AVX instruction set by default (source: https://www.tensorflow.org/install/source#preconfigured_configurations), but it is still possible to build it without AVX support.

The AVX instruction set is available on intel CPUs since 2011 so I guess it's a reasonable expectation to assume that it is available. I don't know if pip will correctly build the wheel from source considering the CPU architecture, but it is definitely possible (although not very user friendly) to build a later version without AVX support.

[kousu]

• Upgrading tensorflow means that SCT is no longer available to users whose CPUs do not support the AVX instruction set.

Just to add a correction here: the pre-built versions of tensorflow > v1.6 are using the AVX instruction set by default (source: https://www.tensorflow.org/install/source#preconfigured_configurations), but it is still possible to build it without AVX support.

The AVX instruction set is available on intel CPUs since 2011 so I guess it's a reasonable expectation to assume that it is available. I don't know if pip will correctly build the wheel from source considering the CPU architecture, but it is definitely possible (although not very user friendly) to build a later version without AVX support.

It's supposed to but it's a mess, especially on Windows and macOS which don't tend to come with compilers installed. Even if you have one it might be an incompatible compiler, and even if you have a compatible one building tensorflow is a multi-hour process, maybe a multi-day process on an older machine. On the dev side, I want to make sure everything can build from source; but on the user side, we need to find a workaround because that's not a reasonable thing to expect out of them.

[Drulex]

• Upgrading tensorflow means that SCT is no longer available to users whose CPUs do not support the AVX instruction set.

Just to add a correction here: the pre-built versions of tensorflow > v1.6 are using the AVX instruction set by default (source: https://www.tensorflow.org/install/source#preconfigured_configurations), but it is still possible to build it without AVX support.
The AVX instruction set is available on intel CPUs since 2011 so I guess it's a reasonable expectation to assume that it is available. I don't know if pip will correctly build the wheel from source considering the CPU architecture, but it is definitely possible (although not very user friendly) to build a later version without AVX support.

It's supposed to but it's a mess, especially on Windows and macOS which don't tend to come with compilers installed. Even if you have one it might be an incompatible compiler, and even if you have a compatible one building tensorflow is a multi-hour process, maybe a multi-day process on an older machine. On the dev side, I want to make sure everything can build from source; but on the user side, we need to find a workaround because that's not a reasonable thing to expect out of them.

Perhaps, but how many people actually use CPUs that don't support AVX? If our friends at google don't build it anymore maybe the occurrence is low.

In any case we can compile and distribute the binaries ourselves if we want to. Just trying to show that pinning tensorflow to a low version is not actually a hard requirement.

[kousu]

Perhaps, but how many people actually use CPUs that don't support AVX? If our friends at google don't build it anymore maybe the occurrence is low.

Enough for this emergency downgrade barely a year ago: #2618

Our userbase is not rich cloud-savvy startups in high-bandwidth zones. Some are on the upside of the digital divide, many more people only get that kind of access indirectly via HPC clusters, and many are just using one old Windows machine in a corner of their lab.

In any case we can compile and distribute the binaries ourselves if we want to. Just trying to show that pinning tensorflow to a low version is not actually a hard requirement.

For sure, this is what I'm advocating. But we've gotta do the work to make that a practical option for the people in the gap. But it's looking like just going to drop tensorflow entirely and hope torch is doing a slightly better job supporting more diverse CPUs.

It's interesting that this is the sort of issue that should be caught by CI but can't be because CI chooses a common denominator. I mean, unless we set up our own fleet of self-hosted CI. But I don't have the capacity to maintain that right now and I don't know who in the lab or at polytechnique would.

[joshuacwnewton]

Given the discussion from today's meeting, it became more clear that the goal will be to prioritize #3046 so that the AVX-related workarounds will hopefully not be necessary.

Still, we can still keep an eye on this in the coming month. This PR can still help us bridge the gap (e.g. maybe by using it to extend the deprecation period for the old methods past December 2021).

[kousu]

Given the discussion from today's meeting, it became more clear that the goal will be to prioritize #3046 so that the AVX-related workarounds will hopefully not be necessary.

Still, we can still keep an eye on this in the coming month. This PR can still help us bridge the gap (e.g. maybe by using it to extend the deprecation period for the old methods past December 2021).

Today at our meeting we figured we are going to miss our deadline. So we need to just upgrade tensorflow in the meantime, and work on doing AVX-less tensorflow builds.

We should be able to do this by setting some ./configure flags, waiting for the build to finish, and uploading the results somewhere. We can get Github Actions to do the builds, once we figure out the right flags to pick. And then our instructions to users will be:

"If you see AVX errors, run: pip install -f https://github.com/spinalcordtoolbox/tensorflow/releases -U tensorflow"

[joshuacwnewton]

"If you see AVX errors, run: pip install -f https://github.com/spinalcordtoolbox/tensorflow/releases -U tensorflow"

Actually, now that I think about it... could we even do this step on behalf of the user during install_sct?

The AVX error occurs immediately when Keras is imported. So, maybe we could do something like:

if [[ $(python -c "import keras") == "*Illegal instruction*" ]]
then
    echo "CPU does not appear to support AVX instruction set. Installing compiled AVX-less version of Tensorflow..."
    pip install -f https://github.com/spinalcordtoolbox/tensorflow/releases -U tensorflow
fi

[kousu]

if [[ $(python -c "import keras") == "*Illegal instruction*" ]]
then
    echo "CPU does not appear to support AVX instruction set. Installing compiled AVX-less version of Tensorflow..."
    pip install -f https://github.com/spinalcordtoolbox/tensorflow/releases -U tensorflow
fi

Huh, yeah, I think that should work :). I like that it only applies to people who need the workaround.

When checking text outputs like this you might need to write "$(LANG=C python -c "import keras" 2>&1)" to be safe (LANG=C means "use American English", because C was developed at Bell labs, in America and 2>&1 means mix stderr into stdout).

[joshuacwnewton]

This PR should be good to go, except for the support for AVX-less users, as discussed above.

I've opened #3509 to document this effort separately, as to avoid muddying up this PR further.

[joshuacwnewton]

I've compiled an AVX-less Tensorflow 1.15.5 wheel (Python 3.7, Linux) here: spinalcordtoolbox/docker-tensorflow-builder#1

I'll wait until @kousu returns from vacation before figuring out whether we want to try to install this for the user in install_sct, or whether we want to let them try to install it themselves (see "Task 3" in issue #3509).

[joshuacwnewton]

In 0c2e7fd, I added a check that should auto-install the AVX-less TensorFlow wheel if an instruction set error is thrown.

I tested the new additions (check for error, install TensorFlow) using the following bash script:

#!/bin/bash
for run in {1..2}; do
  echo "Checking if AVX error is thrown with Tensorflow 1.15"
  if qemu-x86_64 --cpu Westmere-v1 /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/bin/python3 -c "import keras; print(keras.__version__)" 2>&1 | 
     grep -q 'The TensorFlow library was compiled to use'; then
    echo "AVX error was thrown!"
    echo "Installing a version of Tensorflow compiled for older CPUs..."
    source "/home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/etc/profile.d/conda.sh"
    conda activate venv_sct
    pip uninstall tensorflow -y
    pip install https://github.com/spinalcordtoolbox/docker-tensorflow-builder/releases/download/v1.15.5-py3.7/tensorflow-1.15.5-cp37-cp37m-linux_x86_64.whl
  else
    echo "AVX error was not thrown!"
  fi
done

The output of running this script is here:

joshua@tadpole:~/Desktop$ ./test.sh
Checking if AVX error is thrown with Tensorflow 1.15
AVX error was thrown!
Installing a version of Tensorflow compiled for older CPUs...
Found existing installation: tensorflow 1.15.5
Uninstalling tensorflow-1.15.5:
  Successfully uninstalled tensorflow-1.15.5
Collecting tensorflow==1.15.5
  Downloading https://github.com/spinalcordtoolbox/docker-tensorflow-builder/releases/download/v1.15.5-py3.7/tensorflow-1.15.5-cp37-cp37m-linux_x86_64.whl (81.8 MB)
     |████████████████████████████████| 81.8 MB 1.8 MB/s 
Requirement already satisfied: tensorboard<1.16.0,>=1.15.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.15.0)
Requirement already satisfied: h5py<=2.10.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (2.10.0)
Requirement already satisfied: grpcio>=1.8.6 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.41.0)
Requirement already satisfied: opt-einsum>=2.3.2 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (3.3.0)
Requirement already satisfied: wheel>=0.26 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (0.37.0)
Requirement already satisfied: google-pasta>=0.1.6 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (0.2.0)
Requirement already satisfied: astor>=0.6.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (0.8.1)
Requirement already satisfied: six>=1.10.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.16.0)
Requirement already satisfied: protobuf>=3.6.1 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (3.18.1)
Requirement already satisfied: numpy<1.19.0,>=1.16.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.18.5)
Requirement already satisfied: gast==0.2.2 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (0.2.2)
Requirement already satisfied: keras-preprocessing>=1.0.5 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.1.2)
Requirement already satisfied: wrapt>=1.11.1 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.13.2)
Requirement already satisfied: keras-applications>=1.0.8 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.0.8)
Requirement already satisfied: tensorflow-estimator==1.15.1 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.15.1)
Requirement already satisfied: absl-py>=0.7.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (0.15.0)
Requirement already satisfied: termcolor>=1.1.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorflow==1.15.5) (1.1.0)
Requirement already satisfied: markdown>=2.6.8 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (3.3.4)
Requirement already satisfied: setuptools>=41.0.0 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (58.0.4)
Requirement already satisfied: werkzeug>=0.11.15 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (2.0.2)
Requirement already satisfied: importlib-metadata in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from markdown>=2.6.8->tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (4.8.1)
Requirement already satisfied: typing-extensions>=3.6.4 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from importlib-metadata->markdown>=2.6.8->tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (3.10.0.2)
Requirement already satisfied: zipp>=0.5 in /home/joshua/repos/spinalcordtoolbox_test-python-3.7-tensorflow-1.15/python/envs/venv_sct/lib/python3.7/site-packages (from importlib-metadata->markdown>=2.6.8->tensorboard<1.16.0,>=1.15.0->tensorflow==1.15.5) (3.6.0)
Installing collected packages: tensorflow
Successfully installed tensorflow-1.15.5
Checking if AVX error is thrown with Tensorflow 1.15
AVX error was not thrown!

So, it looks like the check and the pip commands work as expected. :)

[kousu]

:)

[joshuacwnewton]

Note that this PR getting merged also unlocks #3340 because of #3340 (comment).

And, finishing that PR is also a boon for #3095, because we'll be able to display the arguments/help descriptions of our CLI tools in a more user-friendly format.