Page MenuHomePhabricator
Create Task
Maniphest T28140

[MultiLabel Segmentation] Default parameter (unsigned int layer = 0) is unnecessary and causes problems
Closed, ResolvedPublic
Actions

Description

Several functions inside mitkLabelSetImage.h use unsigned int layer = 0 as default parameter / value. I remember that this led to a lot of problems:
Developers did not get a compile-error when using no argument (e.g. calling GetActiveLabel()) but the wrong active label is returned, if the user is working on a different layer (e.g. layer 2).
I don't see any necessity to provide a default parameter so I would like to remove it and replace all calls to functions with default parameters to function calls with a specific layer argument.

Related Objects
Search...

StatusAssignedTask
 ResolvedkislinskT28176 [Segmentation] Inspection of module and plugin
 ResolvedkislinskT26959 [Segmentation] Newly created LabelSet segmentation is not displayed as "outline binary"
 ResolvedkislinskT28479 [Segmentation] Rethink / reevaluate our existing segmentation tools / algorithms
 OpenNoneT8140 Reimplement volume annotations for segmentations
 ResolvedkislinskT27319 [Segmentation] Correctly implement interpolation features for Multilabel Segmentation View
 ResolvedflocaT29132 [MultiLabel Segmentation] Define desired behavior of fill / erase tool
 ResolvedflocaT28145 SetRegionTool does not work as expected (flood fill like) with concentric structures.
 ResolvedkislinskT27807 [MultiLabel Segmentation] Inspection of module and plugin
 WontfixNoneT22593 Update the MitkMultilabelTestDriver
 WontfixNoneT22458 Merging a label in the MultiLabel segmentation plugin leaves one label without a segmentation
 OpenNoneT26397 Multilabel Segmentation file cannot be converted to multilabel segmentation in MITK
 ResolvedflocaT23981 Multilabel color map should disable interpolation
 ResolvedkislinskT25445 MultiLabel segmentation mapper is very slow
 ResolvedkalaliT28131 [MultiLabel Segmentation] 3D Threshold tool does not work with multiple labels / layers
 ResolvedflocaT28526 [Segmentation] Check tools (especially 3D tools) to correctly handle updating of LabelSetImage
 OpenflocaT28524 Refactor LabelSetImage and associated classes.
 ResolvedflocaT28525 Design refactoring of LabelSetImage and associated classes
 ResolvedflocaT28140 [MultiLabel Segmentation] Default parameter (unsigned int layer = 0) is unnecessary and causes problems

Event Timeline

kalali triaged this task as Normal priority.Jan 3 2021, 4:04 PM
kalali created this task.
floca edited projects, added MITK (v2021.10), Request for Discussion; removed Next Milestone, MITK.Apr 9 2021, 6:49 PM
floca moved this task from Backlog to Segmentation on the MITK (v2021.10) board.Apr 9 2021, 6:59 PM
floca added a parent task: T28525: Design refactoring of LabelSetImage and associated classes.Jul 27 2021, 9:57 AM
kislinsk edited projects, added MITK (v2022.04); removed MITK (v2021.10).Nov 10 2021, 4:32 PM
floca moved this task from Backlog to Segmentation on the MITK (v2022.04) board.Nov 17 2021, 11:59 AM
kalali edited projects, added MITK, Next Milestone; removed MITK (v2022.04).Apr 20 2022, 4:52 PM
kalali added a comment.Apr 28 2022, 4:15 PM

Just out of curiosity I removed all default parameters for the following functions:

  • mitkLabelSetImage::MergeLabel
  • mitk::LabelSetImage::MergeLabels
  • mitk::LabelSetImage::UpdateCenterOfMass
  • mitk::LabelSetImage::RemoveLabels
  • mitk::LabelSetImage::EraseLabel
  • mitk::LabelSetImage::EraseLabels
  • mitk::LabelSetImage::GetActiveLabel
  • const mitk::LabelSetImage::GetActiveLabel
  • mitk::LabelSetImage::GetLabel
  • mitk::LabelSetImage::GetLabelSet
  • const mitk::LabelSetImage::GetLabelSet
  • mitk::LabelSetImage::GetNumberOfLabels
  • mitk::LabelSetImage::CreateLabelMask

And got the following errors:

image.png (774×2 px, 133 KB)

kalali edited projects, added MITK (v2022.10); removed Next Milestone, MITK.May 12 2022, 5:15 PM
kalali moved this task from Backlog to Segmentation on the MITK (v2022.10) board.
kalali added subscribers: floca, kislinsk.May 17 2022, 6:07 PM

I just stumbled upon the following:

Inside of the LabelSetImagethere are two functions (four with const):

  • mitk::Label *GetActiveLabel(unsigned int layer = 0);
  • const mitk::Label* GetActiveLabel(unsigned int layer = 0) const;

and

  • mitk::LabelSet *GetActiveLabelSet();
  • const mitk::LabelSet* GetActiveLabelSet() const;

Inside the GetActiveLabelSet-functions to access the vector GetActiveLayer is always used. This makes sense as the name of the function suggests.
Inside the GetActivelabel-functions to access the vector the given (possibly default) argument is used. This does not make sense and is counterintuitive. This is a special case where the default layer parameter is not only unnecessary but completely misleading. I have never seen a call to GetActiveLabel without the typical argument (GetActiveLabel(GetActiveLayer()), so removing the default parameter here will probably do no harm but remove obscurity. Since it was said to not work too much on the LabelSetImage, I just ping @floca and @kislinsk here. At least this should be considered for a new data structure, but it could also be fixed quickly.

floca added a comment.May 17 2022, 8:03 PM

in the new data structure it won't happen as there is no active label anymore. But you are right I thought the same when I looked through the class. My motiviation to refine this obsolete class was only limited 🙈 .

kislinsk edited projects, added MITK (v2023.04); removed MITK (v2022.10).Nov 23 2022, 3:52 PM
floca moved this task from Backlog to Segmentation on the MITK (v2023.04) board.Nov 24 2022, 2:36 PM
floca removed a project: Request for Discussion.Jan 25 2023, 9:27 AM
kislinsk edited projects, added MITK (v2023.12); removed MITK (v2023.04).May 17 2023, 2:55 PM
kislinsk edited projects, added MITK (v2024.06); removed MITK (v2023.12).Dec 5 2023, 10:38 AM
floca closed this task as Resolved.Feb 22 2024, 5:26 PM
floca claimed this task.

Will be fixed with T30294.

MITK · MIC · IMSY · DKFZ