Make sure to produce JSON files with appropriate target_suffix to be used by 3rd party software

[jcohenadad]

The default JSON file output from the model has the target_suffix "_seg-manual" (for SC segmentation), which is a problem because it is counter-intuitive.

A solution is to change this field for something more intuitive, eg: "_seg".

I've done it for the release https://github.com/ivadomed/model_seg_ms_mp2rage/releases/tag/r20211223 (ie: i replaced the ZIP files of both models). Here are fields I used:

For model_seg_ms_lesion_mp2rage.json:

        "target_suffix": [
            "_lesion"
        ],

For model_seg_ms_sc_mp2rage:

      "target_suffix": [
            "_seg"
        ],

Related to #41

[mariehbourget]

@jcohenadad,
I came across this issue and I think there may be a misunderstanding on the new naming convention in ivadomed and the usage of target_suffix.

IIUC, you changed the target_suffix in order to name the prediction files correctly.

In ivadomed, the target_suffix in the config file are used to train the model on the "right" derivatives and are not used anymore for naming the prediction files (as of recent changes in ivadomed/ivadomed#1043).
By changing the field in the packaged model, you won't be able to re-train the model from this config file in the future because the target_suffix won't match the derivatives files.

In our discussions and in ADS meeting, we agreed that the suffix for the prediction filenames should be determined on ADS side, independent from the ivadomed config file.
(see slides from last ADS meeting: https://docs.google.com/presentation/d/1wxBsS-X2fVDEc--sGLEHBXrXb27yjGw1d0IiMUIBqq8/edit#slide=id.g10c5c80ed37_0_28)

My understanding was that a similar mechanism would be implemented in SCT as well to name the prediction files without using the target_lst from ivadomed. Currenlty sct_deepseg uses the target_lst from ivadomed: https://github.com/spinalcordtoolbox/spinalcordtoolbox/blob/f5aa4ce78077e44511f7e5fc923e8c2eda438ae8/spinalcordtoolbox/scripts/sct_deepseg.py#L216), whereas ADS will use its own suffixes (implementation in-progress as per the slides).

[jcohenadad]

Thank you very much for the clarification, @mariehbourget. Indeed, I wasn't sure what we agreed on. I thought that "target_suffix" should be disregarded only for multiclass predictions, but now I understand that it should always be ignored for any predictions (which is better, as it is less confusing).

We could indeed add lines of code in SCT to rename the output prediction. I'll see how this could be done elegantly across all models. Thank you again for your inputs!!

[uzaymacar]

Completely agree with the conclusion here! To further motivate, two different projects can use the same "target_suffix" (e.g. lesion-manual) to load the corresponding derivatives, but their prediction filenames should probably be different when used in deployment. Therefore, it makes sense for any 3rd-party software to decide their own naming convention when it comes to output files. It is almost like an external "prediction_suffix" as discussed here: ivadomed/ivadomed#1006.