lassoan / SlicerMONAIAuto3DSeg

Extension for 3D Slicer for running MONAI Auto3DSeg models
MIT License
66 stars 10 forks source link

Add flag for BRATS inference #43

Closed diazandr3s closed 7 months ago

diazandr3s commented 7 months ago

It is quite frustrating already that BRATS causes so much extra work and complexity. We should not let it creep out of the inference script.

We already have if save_mode == 'brats23' or 'brats23' in config['bundle_root']: # special case brats in the inference script. Do we really need this special case here? Why the inference script cannot determine if it is dealing with a brats model?

Thanks for checking this, @lassoan. I've simplified the save_mode flag management. This argument ( save_mode) has been part of the inference script since we created it. It is for special cases in which the labels/segments overlap.

lassoan commented 7 months ago

It is for special cases in which the labels/segments overlap.

Overlapping segments is not a special case and we need to provide a standard solution. However, this should not be called "brats" but something more generic (segment_overlap_allowed = true/false). If what brats does is a good way to do it, then we just need to rename the option, specify what we mean by it, and we are done.

However, as I remember, brats is also special in how it receives the input grayscale images. Does brats serve as a good example? Would we encourage others to take in multiple input images like brats does it? If yes, then give it a good name, specify what that name means, and we are good.

If brats requirements for inputs or outputs do not make sense then we should change the models to use better, standard methods. If switching to better methods is not that easy to do it then adding special cases in the inference script is an acceptable workaround. But these special cases should be confined to the inference script (must not leak out to the application layer, like a Slicer module script).

diazandr3s commented 7 months ago

It is for special cases in which the labels/segments overlap.

Overlapping segments is not a special case and we need to provide a standard solution. However, this should not be called "brats" but something more generic (segment_overlap_allowed = true/false). If what brats does is a good way to do it, then we just need to rename the option, specify what we mean by it, and we are done.

However, as I remember, brats is also special in how it receives the input grayscale images. Does brats serve as a good example? Would we encourage others to take in multiple input images like brats does it? If yes, then give it a good name, specify what that name means, and we are good.

If brats requirements for inputs or outputs do not make sense then we should change the models to use better, standard methods. If switching to better methods is not that easy to do it then adding special cases in the inference script is an acceptable workaround. But these special cases should be confined to the inference script (must not leak out to the application layer, like a Slicer module script).

Hi @lassoan,

Many thanks again for your input.

You're right we must not leak out to the application layer. I've just updated the script.