-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SegmentHumanBody extension #2014
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
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.
@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. |
@jamesobutler I've closed the last issue as well |
@jamesobutler Could you find a chance to check the recent changes in the repository? |
@jamesobutler I'm writing to follow up. Do you have more reviews about the extension? |
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:
|
Thank you for your submission. I had a look at code and found two issues:
|
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. |
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. |
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
and
|
Agreed. Restrictions on the source data and the trained model should be both clearly described. |
Thank you all for the discussion above. @jcfr I run the commands you provided and pushed the changes. @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. |
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. |
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)? |
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. |
New extension
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 enter3d-slicer-extension
in "Topics" and click "Save changes". To learn more about topics, read https://help.github.com/en/articles/about-topicsSettings
and in repository settings uncheckWiki
,Projects
, andDiscussions
(if they are currently not used)About
in the top-right corner of the repository main page and uncheckReleases
andPackages
(if they are currently not used)