Skip to content
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

Open
wants to merge 70 commits into
base: main
Choose a base branch
from

Conversation

FJDorfner
Copy link

Pull Request to add the MRSegmentator model to mhub.

Copy link
Member

@LennyN95 LennyN95 left a 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",
Copy link
Member

@LennyN95 LennyN95 Jul 7, 2024

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 ;))

Copy link
Member

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?

Copy link
Author

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:)

Screenshot 2024-07-24 at 10 24 47

"label": "Input Image",
"description": "The MR or CT scan of a patient (Thorax, Abdomen and Pelvis).",
"format": "DICOM",
"modality": "MRI|CT",
Copy link
Member

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.

@LennyN95
Copy link
Member

@FJDorfner since all comments from the previous review are now implemented, we can continue with the testing procedure.

@FJDorfner
Copy link
Author

@LennyN95 Thanks! I will continue with the testing.

@LennyN95
Copy link
Member

Please note, we updated our base image. All mhub dependencies are now installed in a virtual environment under /app/.venv running Python 3.11. Python, virtual environment and dependencies are now managed with uv. If required, you can create custom virtual environments, e.g., uv venv -p 3.8 .venv38 and use uv pip install -p .venv38 packge-name to install dependencies and uv run -p .venv3.8 python script.py to run a python script.

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.

Copy link
Member

@LennyN95 LennyN95 left a 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 ;)

@LennyN95
Copy link
Member

Static Badge

Test Results
id: 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

@FJDorfner
Copy link
Author

Hey @LennyN95, thank you for your help with the contribution process!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Testing
Development

Successfully merging this pull request may close these issues.

2 participants