-
Notifications
You must be signed in to change notification settings - Fork 18
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
[PW41] Add MRSegmentator Model #90
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.
Great Work!!
I suggested some minor changes. I'll update segdb and once we have that we can continue with the test routine. Thx @FJDorfner, very clean and to the point :)
{ | ||
"type": "Segmentation", | ||
"classes": [ | ||
"SPLEEN", |
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'll add SPINE
to SegDB. I can lookup the codes, but feel free to add SCT code's if you already have them ;))
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.
@FJDorfner We're adding a quite broad label here, can you explain weather the model delineates the entire spine or the bone-structure of the spine?
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.
Hi @LennyN95, the model segments the bodies of the vertebrae individually. The disk between is not included in the label. This would be the yellow label in the attached Screenshot as a visual:)

"label": "Input Image", | ||
"description": "The MR or CT scan of a patient (Thorax, Abdomen and Pelvis).", | ||
"format": "DICOM", | ||
"modality": "MRI|CT", |
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.
- TODO @LennyN95: Add support for multi-modality inputs. Dash separation is a good idea and in line with DTQ syntax.
@FJDorfner since all comments from the previous review are now implemented, we can continue with the testing procedure. |
@LennyN95 Thanks! I will continue with the testing. |
Please note, we updated our base image. All mhub dependencies are now installed in a virtual environment under We also simplified our test routine. Sample and reference data now have to be uploaded to Zenodo and provided in a mhub.tom file at the project root. The process how to create and provide these sample data is explained in the updated testing phase article of our documentation. Under doi.org/10.5281/zenodo.13785615 we provide sample data as a reference. |
…ix batchgenerators
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.
Thank you @FJDorfner for fixing the issues. Looks good!
I started the testing on this PR ;)
Test Resultsid: e458deff-2799-47a1-9ac9-90f40b6e7573
name: MHub Test Report (default)
date: '2025-03-25 11:45:30'
checked_files:
- file: MRSegmentator.seg.dcm
path: /app/data/output_data/1.3.6.1.4.1.14519.5.2.1.1.34685322639756871092106477180544961507/MRSegmentator.seg.dcm
checks:
- checker: DicomsegContentCheck
notes:
- label: Segment Count
description: The number of segments identified in the inspected dicomseg file.
info: 32
summary:
files_missing: 0
files_extra: 0
checks:
DicomsegContentCheck:
files: 1
conclusion: true
|
Hey @LennyN95, thank you for your help with the contribution process! |
Pull Request to add the MRSegmentator model to mhub.