Deprecate Tensorflow-based methods (sct_deepseg_<x>) and use only PyTorch (sct_deepseg)

[joshuacwnewton]

Context

Historically, SCT has used Keras/tensorflow for its deep learning-based segmentation methods (sct_deepseg_sc, sct_deepseg_gm, sct_deepseg_lesion). However, #2639 introduced the more general sct_deepseg, which aims to provide alternatives that use pytorch instead. (More context in #2626.)

But, as noted in #2639, the transition is not complete:

For now, this new function will not replace sct_deepseg_X, but will be complementary to the existing functions, so we can gather feedback from users before making these functions deprecated. The "big" change will likely be associated with a major release.

Motivation

The main motivation to switch is to unify the development between SCT and ivadomed. Further information here: #2626 (comment)

Other benefits include:

• Easier Python version support, because Tensorflow is typically the only blocker for upgrading Python versions:

  • tensorflow==1.5 ties us to Python 3.6, which is quickly approaching EOL (#3227).
  • tensorflow==1.15 ties us to Python 3.7. (#3367)
  • etc.

  Since PyTorch is the main library used by sister project ivadomed, and their development team has already spent much effort testing Python 3.7/3.8/3.9, becoming PyTorch-only helps to sync us up with ivadomed.

• Fewer dependencies, so fewer upstream issues (e.g. #2987, #2562)

• Security benefits (we're currently quietly ignoring many dependabot vulnerability warnings for Tensorflow.)

Next steps

There is a brief summary in #3227 (comment):

The bigger work is really to tie in the new model training with the git-annex datasets that @kousu is deploying. We also need to make some decisions on how we want to expose the models, what default models to install, how many flavors of these models (eg: SoftSeg vs. HardSeg), etc.

A more in-depth summary is provided here: https://gist.github.com/joshuacwnewton/c59f1aa0d805e7e2d912eafd99e38c6a

[kousu]

Related: #3088

[kousu]

We talked at the weekly ivadomed meeting about this. My understanding of our conclusion is:

1. There are efforts to train newer, better models

   1. sct_deepseg_gm has no active project that could replace it
   2. sct_deepseg_lesion: There is a project to make a better multiple-sclerosis lesion model (https://github.com/ivadomed/ms-challenge-2021/) as part of ivadomed
   3. sct_deepseg_sc: There is a project to make a contrast agnostic spine model (Train new model for spinal cord segmentation ivadomed/ivadomed#495, https://github.com/sct-pipeline/contrast-agnostic-softseg-spinalcord) as part of ivadomed

   These models are probably not going to be drop-in replacements. They will change the data type of input and output (e.g. going from binary to softseg, straightened vs unstraightened). They are also active research projects with research timelines, performance measurements and papers to be published. They won't be ready soon.

2. There is support to do a shorter-term solution that does a more direct port. Either:

   1. Retrain on pytorch. Find the existing training data, scripts and their existing outputs:

      1. sct_deepseg_gm:
         • data: ?????
         • scripts: https://github.com/neuropoly/gmseg
         • outputs: https://github.com/spinalcordtoolbox/deepseg_gm_models
      2. sct_deepseg_lesion:
         • data: ?????
         • scripts: https://github.com/sct-pipeline/deepseg-training
         • outputs: https://github.com/spinalcordtoolbox/deepseg_lesion_models (@charleygros's work)
      3. sct_deepseg_sc:
         • data: ?????
         • scripts: ?????
         • outputs: https://github.com/spinalcordtoolbox/deepseg_sc_models (@charleygros's work)

      and port the necessary parts to pytorch. Retrain on the same data.

      Should be drop-in compatible since the input shapes are the same.

      Should be able to add these to ivadomed.

   2. Translate tensorflow -> ONNX (Remove dependency on Keras/Tensorflow by relying only on onnxruntime and ONNX models #3735)

      • Should be drop-in compatible
      • From @joshuacwnewton's discovery of: https://github.com/onnx/tensorflow-onnx
      • (related: pytorch -> ONNX Remove dependency on PyTorch by relying only on onnxruntime and ONNX models #3396)
      • Replaces tensorflow with onnx instead of unifying on a single dependency; but it's a smaller, less cranky dependency?
      • https://pypi.org/project/onnxruntime-gpu/ exists to handle inference on GPUs
      • Might not be fully compatible with existing models; tensorflow (and torch) support more complicated neural network architectures (i.e. neural network programs); onnx is a restricted VM
      • Would not add these to ivadomed

   3. Translate tensorflow -> pytorch

      • Should be drop-in compatible!
      • Could be added to ivadomed directly
      • From @joshuacwnewton's discovery of: https://github.com/onnx/tensorflow-onnx + either {https://github.com/ToriML/onnx2pytorch, https://github.com/pytorch/pytorch/issues/21683}

[kousu]

@jcohenadad, please help me fill in the ????? above.

[joshuacwnewton]

The original issue description highlights a number of benefits for moving away from Tensorflow:

• Easier Python version support, because Tensorflow is typically the only blocker for upgrading Python versions:

  • tensorflow==1.5 ties us to Python 3.6, which is quickly approaching EOL (#3227).
  • tensorflow==1.15 ties us to Python 3.7. (#3367)
  • etc.

  Since PyTorch is the main library used by sister project ivadomed, and their development team has already spent much effort testing Python 3.7/3.8/3.9, becoming PyTorch-only helps to sync us up with ivadomed.

• Fewer dependencies, so fewer upstream issues (e.g. #2987, #2562)

• Security benefits (we're currently quietly ignoring many dependabot vulnerability warnings for Tensorflow.)

However, since the migration from Tensorflow to ONNX (#3735), we no longer struggle with any of these issues.

This means that there is much less motivation to retrain the sct_deepseg_<x> models using PyTorch. (I think the main benefit would be to unify the deepseg functions under a single umbrella, but this is more of a naming/organizational benefit than a functional benefit.)

I think, given this, it might be safe to close this issue in its entirety.

I could still recreate the issue in neuropoly/idea-projects as a project idea for future interns/students in the lab (as per #3947 and #3088 (comment)), but I'm not sure how high-priority such a task would be. (To me, it seems relatively low-priority.)