Add SegmentHumanBody extension

[Zaferyildiz]

New extension

• Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
• Repository name is Slicer+ExtensionName
• Repository is associated with 3d-slicer-extension GitHub topic so that it is listed here. To edit topics, click the settings icon in the right side of "About" section header and enter 3d-slicer-extension in "Topics" and click "Save changes". To learn more about topics, read https://help.github.com/en/articles/about-topics
• Extension description summarizes in 1-2 sentences what the extension is usable (should be understandable for non-experts)
• Any known related patents must be mentioned in the extension description.
• LICENSE.txt is present in the repository root. MIT (https://choosealicense.com/licenses/mit/) or Apache (https://choosealicense.com/licenses/apache-2.0/) license is recommended. If source code license is more restrictive for users than MIT, BSD, Apache, or 3D Slicer license then the name of the used license must be mentioned in the extension description.
• Extension URL and revision (scmurl, scmrevision) is correct, consider using a branch name (main, release, ...) instead of a specific git hash to avoid re-submitting pull request whenever the extension is updated
• Extension icon URL is correct (do not use the icon's webpage but the raw data download URL that you get from the download button - it should look something like this: https://raw.githubusercontent.com/user/repo/main/SomeIcon.png)
• Screenshot URLs (screenshoturls) are correct, contains at least one
• Homepage URL points to valid webpage containing the following:
  • Extension name
  • Short description: 1-2 sentences, which summarizes what the extension is usable for
  • At least one nice, informative image, that illustrates what the extension can do. It may be a screenshot.
  • Description of contained modules: at one sentence for each module
  • Tutorial: step-by-step description of at least the most typical use case, include a few screenshots, provide download links to sample input data set
  • Publication: link to publication and/or to PubMed reference (if available)
  • License: We suggest you use a permissive license that includes patent and contribution clauses. This will help protect developers and ensure the code remains freely available. We suggest you use the Slicer License or the Apache 2.0. Always mention in your README file the license you have chosen. If you choose a different license, explain why to the extension maintainers. Depending on the license we may not be able to host your work. Read here to learn more about licenses.
  • Content of submitted s4ext file is consistent with the top-level CMakeLists.txt file in the repository (description, URLs, dependencies, etc. are the same)
• Hide unused features in the repository to reduce noise/irrelevant information:
  • Click Settings and in repository settings uncheck Wiki, Projects, and Discussions (if they are currently not used)
  • Click the settings icon next to About in the top-right corner of the repository main page and uncheck Releases and Packages (if they are currently not used)

[jamesobutler]

I have provided an initial pass of review and have issued

Usability issues:

• Support for Pytorch 2.1 and newer mazurowski-lab/SlicerSegmentHumanBody#1
• Failure to enter SegmentAny3D module mazurowski-lab/SlicerSegmentHumanBody#2

Packaging for ExtensionsIndex distribution issues:

• Packaging not working for distribution on Slicer ExtensionsIndex mazurowski-lab/SlicerSegmentHumanBody#3

Also will need to be mindful if #2011 is integrated before this PR which switches from .s4ext to .json files. See https://discourse.slicer.org/t/introduction-of-tiers-for-slicer-extensions/34870 for more details.

[Zaferyildiz]

@jamesobutler Thank you for your initial review. I fixed 2 of the problems and left a comment about the last one. I also renamed the extension with the last commit.

[Zaferyildiz]

@jamesobutler I've closed the last issue as well

[Zaferyildiz]

@jamesobutler Could you find a chance to check the recent changes in the repository?

[Zaferyildiz]

@jamesobutler I'm writing to follow up. Do you have more reviews about the extension?

[jcfr]

Unable to force push changes onto mazurowski-lab/SlicerExtensionsIndex@main, the updated changes are published here main...jcfr:ExtensionsIndex:mazurowski-lab-main

Consider running the following command to update this pull request:

git clone [email protected]:mazurowski-lab/SlicerExtensionsIndex.git
cd SlicerExtensionsIndex
git checkout main
git remote add jcfr https://github.com/jcfr/ExtensionsIndex.git
git fetch jcfr
git reset --hard jcfr/mazurowski-lab-main
git push origin main --force

[lassoan]

Thank you for your submission. I had a look at code and found two issues:

• The module should not automatically download a huge package such as pytorch without user consent. Please replace the delay display call by a confirm dialog at https://github.com/mazurowski-lab/SlicerSegmentHumanBody/blob/2496837be2a5b42bd92525e246b32f0197633de7/SegmentHumanBody/SegmentHumanBody.py#L101
• Currently, all Slicer extensions use a permissive license (freely usable for any purpose). Usage of non-commercial licenses are not banned, but strongly discouraged, as most users would not want to spend time with even trying a tool that is cannot be used for treating patients, planning surgeries, etc. (these may all be classified as commercial activities). I would recommend to reconsider the license choice. If it remains non-commercial then very visible warnings must be added to the extension description, documentation, and maybe even in the module GUI, to ensure that users are aware that usage of this extension is restricted.

[fedorov]

Related to this, I would like to point out that this extension wraps https://github.com/mazurowski-lab/SegmentAnyBone, which is distributed under CC-BY-NC, which places restrictions on the "derived work".

In this issue mazurowski-lab/SegmentAnyBone#3 I asked what are the implications of this license on the segmentations produced by the model, but did not receive any response.

I think it is very important that there is a clear statement about whether segmentations produced by this model are considered derived work and are covered by the same CC-BY-NC. If this is the case, there should be a very prominent warning about this in the module GUI, as well as a console warning.

[lassoan]

We went through a similar exercise in MONAI Auto3DSeg extension and decided to drop all training data that was distributed with restrictive licenses. Maybe not everyone will make the same conclusions, but we are really happy that we decided so, because it is just so much simpler to avoid such data and not worry about possible implications of using them. Since then, several clinicians approached us and offered to contribute segmentations for free without restrictions, so I'm quite optimistic that this will all work out well.

I agree that if some extensions with restrictive licenses get into the Slicer extensions index then we need to very clearly label them to make sure users are aware of the limitations.

[fedorov]

We went through a similar exercise in MONAI Auto3DSeg extension and decided to drop all training data that was distributed with restrictive licenses.

This is a related decision, and I am indeed glad you did it that way, but the license covering the data does not necessarily determine the license of the model. One could take data available under CC-BY, and (for example) train a model and distribute it under CC-BY-NC, while attributing the original dataset. I don't think there is anything to prevent this. That is why I think it is important to

1. have clarity on the license covering the model

and

2. have clarity on how the license covering the model applies/propagates to the data generated using the model

[lassoan]

Agreed. Restrictions on the source data and the trained model should be both clearly described.

[Zaferyildiz]

Thank you all for the discussion above.

@jcfr I run the commands you provided and pushed the changes.
@lassoan I changed the delay display function to confirm dialog.

@lassoan & @fedorov We appreciate the discussion for the license. However, the project leadership decided to go with CC BY-NC 4.0 license because of commercialization reasons. The readme file of the repository has the license information. Furthermore, a pop-up warning that says the extension and the model has CC BY-NC 4.0 license appears whenever a user opens the extension on 3D Slicer.

We also appreciate the questions about the license, and its downstream consequences. However, we also do not have the legal background to authoritatively answer these questions, similarly with the reviewers.

[Zaferyildiz]

@jcfr @lassoan @fedorov

I'm writing to follow up. Is there any more concerns we need to address?

[fedorov]

We also appreciate the questions about the license, and its downstream consequences. However, we also do not have the legal background to authoritatively answer these questions, similarly with the reviewers.

Presumably, you are with the institution that has legal authority to distribute this code. There is no one else better equipped to answer this question than your legal experts at your institution.

If the questions about the license covering output produced by the model cannot be answered, I do not think it is ethical to distribute this model in Slicer. The users of the model most definitely have the right to know what they are agreeing to. It is the responsibility of the maintainers of Slicer to protect the users of the application.

I am not the one making the decisions here, but as a reviewer, I personally would vote against including a model if the terms of use for its output are unknown.

[lassoan]

I agree that we must avoid misleading users, but as long as we can make sure users are aware of all limitations, we should be all good. We'll soon also have classification of extensions into tiers, which should further help preventing users from accidentally using extensions with restrictive license.

The license information "The repository is licensed under the CC BY-NC 4.0" is quite prominently displayed on the extension main page. Since there is a direct link to information (and mostly developers visit the github page), I think the terms are clear enough.

Within the module in Slicer, a popup is displayed with this text: "The model and the extension is licensed under the CC BY-NC 4.0 license!\n\nPlease also note that this software is developed for research purposes and is not intended for clinical use yet. Users should exercise caution and are advised against employing it immediately in clinical or medical settings." I think this should be sufficient.

The only place where the unusual license is not displayed is in the extension description. Maybe we could add a sentence like "CC BY-NC 4.0 license. Commercial use is not allowed" to avoid disappointing users (users only realize that they cannot use the after they already installed it)?

@pieper @jcfr what do you think?

[pieper]

I'm wondering if we should have a special tier, or maybe a tier modifier for any non-commercial extensions.

[lassoan]

I'm wondering if we should have a special tier, or maybe a tier modifier for any non-commercial extensions.

I think we can keep it simple and say that requirement for tier 3 or better is non-restrictive license.

[Zaferyildiz]

@lassoan I also added a text about the license to extension description and the issue you have opened was closed. Thank you for the discussion.

[Zaferyildiz]

@lassoan @pieper @jcfr
Hi, I was on vacation for a month and I am writing to follow up. Is there anything else we can help with the extension integration?

[Zaferyildiz]

Hi all, I'm writing to follow up. Is there any updates for our pull request?

[Zaferyildiz]

Hi, could you find a chance to read my previous comments?

[Zaferyildiz]

Hi all, I'm writing to follow up. Could you take a look at the recent changes?

[Zaferyildiz]

Hi all, I'm writing to follow up. Are there any updates to our pull request?

[lassoan]

Thank you for your patience with this.

I've tried the extension. The documentation was very helpful in getting started, but unfortunately I was not able to get any usable segmentation result from it.

I've loaded CTLiver image from Sample Data module, created two segments (hip and femur), and clicked "Run Automatic Segmentation" button and got this very poor result after 12 minutes:

Is this expected? Is there any other Slicer sample data set where the segmentation result is more reasonable?

I've tried the "Assign label (2D)" and "Assign label (3D)" buttons, but they did not have any effect, regardless of what label and mask I chose. I then created box prompts and placed positive and negative prompt points, clicked "Start segmentation for current slice", but either nothing happened or the whole slice got filled.

These errors appeared in the Python console:

>>> 
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 550, in onAssignLabel2D
    currentMask = self.bfs(currentMask, promptPointToAssignLabel)
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 596, in bfs
    targetValue = mask[promptPointToAssignLabel[0], promptPointToAssignLabel[1]]
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 464, in onAssignLabelIn3D
    elif self.sliceAccessorDimension == 0 and self.segmentIdToSegmentationMask[self.firstSegmentId][sliceIndex,:,:][promptPointToAssignLabel[1], promptPointToAssignLabel[0]]:
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 464, in onAssignLabelIn3D
    elif self.sliceAccessorDimension == 0 and self.segmentIdToSegmentationMask[self.firstSegmentId][sliceIndex,:,:][promptPointToAssignLabel[1], promptPointToAssignLabel[0]]:
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 550, in onAssignLabel2D
    currentMask = self.bfs(currentMask, promptPointToAssignLabel)
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 596, in bfs
    targetValue = mask[promptPointToAssignLabel[0], promptPointToAssignLabel[1]]
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 550, in onAssignLabel2D
    currentMask = self.bfs(currentMask, promptPointToAssignLabel)
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 596, in bfs
    targetValue = mask[promptPointToAssignLabel[0], promptPointToAssignLabel[1]]
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 464, in onAssignLabelIn3D
    elif self.sliceAccessorDimension == 0 and self.segmentIdToSegmentationMask[self.firstSegmentId][sliceIndex,:,:][promptPointToAssignLabel[1], promptPointToAssignLabel[0]]:
IndexError: index 635 is out of bounds for axis 1 with size 512
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 738, in onStartSegmentation
    self.collectPromptInputsAndPredictSegmentationMask()
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 941, in collectPromptInputsAndPredictSegmentationMask
    self.producedMask = self.masks[2][:]
IndexError: index 2 is out of bounds for axis 0 with size 2
Traceback (most recent call last):
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 738, in onStartSegmentation
    self.collectPromptInputsAndPredictSegmentationMask()
  File "C:/D/SlicerSegmentHumanBody/SegmentHumanBody/SegmentHumanBody.py", line 941, in collectPromptInputsAndPredictSegmentationMask
    self.producedMask = self.masks[2][:]
IndexError: index 2 is out of bounds for axis 0 with size 2

I've also received many of these warnings:

[VTK] Warning: In vtkSlicerSegmentationsModuleLogic.cxx, line 1715
[VTK] vtkMRMLSegmentationNode (000002765B613020): vtkSlicerSegmentationsModuleLogic::ImportLabelmapToSegmentationNode: Segmentation is a floating point scalar type and will be cast to an integer type. Voxel values may be truncated

These are due to loading floating-point voxel values into a segmentation. To fix it, you can cast the segmentation result to integer (e.g., uint8) at the end of inference, before writing to file.

I've updated your documentation (see mazurowski-lab/SlicerSegmentHumanBody#6) to make it a bit easier to follow (step-by-step instructions). Please improve it further: describe the steps that lead to a usable segmentation result (choose an image on that the model actually works, describe more clearly how to relabel data and how to use prompts - exactly what buttons have to be clicked in what order).

I also find the extension name claiming too much, which is unfair to other extension developers and may be confusing for users. "Segment" word is fine, but then it has to be made more specific, it has to be distinguished from other extensions in a truthful and meaningful way. "HumanBody" does not seem to be valid, as it does not segment any parts of the body and it is not restricted to the human body. The extension uses SAM or SegmentAnyBone and can do automatic segmentation in 3D, so names like "SegmentWithSAM3D" or "SegmentAnyBone" would work better, at least it would be a bit more specific, and if people know SegmentAnythingModel or SegmentAnyBone model then they would know that this extension exposes these models.

[jamesobutler]

I've loaded CTLiver image from Sample Data module, created two segments (hip and femur), and clicked "Run Automatic Segmentation" button and got this very poor result after 12 minutes:

@lassoan Maybe your poor results using the CTLiver sample dataset is because that is a CT volume? https://github.com/mazurowski-lab/SegmentAnyBone specifically says it is for MRI volumes. This extension (https://github.com/mazurowski-lab/SlicerSegmentHumanBody/blob/main/README.md) doesn’t explicitly describe whether it is MRI only or if it is possible to segment CT volumes as well.

[lassoan]

The screenshot in the documentation shows a CT image - https://github.com/mazurowski-lab/SlicerSegmentHumanBody/blob/main/README.md#license

The documentation does not mention what modality and/or what kind of structures it can segment.

This definitely need to be described in the documentation, and maybe included also in the extension name (something like SegmentAnyBoneMR).

[fedorov]

The screenshot in the documentation shows a CT image - https://github.com/mazurowski-lab/SlicerSegmentHumanBody/blob/main/README.md#license

Same picture is also used in https://github.com/mazurowski-lab/SlicerSegmentWithSAM, so I am not sure if it is intended to convey anything specific to the repo functionality.

[Zaferyildiz]

Thanks for your reviews. SegmentAnyBone is developed only for segmentation of MRIs. It is probably the reason why you got a poor segmentation results since you tried to run it on CT image. However, we plan to add more models that can segment other type of tissues in the future. That is why we wanted to call it in a more generalizable way (e.g. SegmentHumanBody).

• I did some performance improvements for the automatic segmentation mode. It should not take 12 minutes now.
• I have changed the logo of the extension to a segmented human body animation. I think it reflects the extension more than before.
• I could not experience the errors you got with CTLiver image, but for the label assignment did you place a prompt point on the object of interest whose label you want to change before you click "Assign Label" button?
• I have added more details about the usage of the extension, especially for the label assignment part. I'm also working on a video recording that shows how to use different features of the extension.

[Zaferyildiz]

Hi, could you try to run it on a MRI sample and check my previous comment?

[Zaferyildiz]

Hi all, I'm writing to follow up

[Zaferyildiz]

@lassoan @jamesobutler We think that we have addressed all the concerns. Is there anything else we need to address?

[Zaferyildiz]

Is there anything else we need to address?

[Zaferyildiz]

Hi all, is there anything else we need to address?

[Zaferyildiz]

Hi all, is there anything else we need to address?

[Zaferyildiz]

Hi all, is there anything else that is preventing us from moving forward?

[lassoan]

Thank you for your patience. We are very busy with preparing the new Slicer Stable Release (5.8). I'll try to get back to this issue next week.