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

Improve conversion of DICOM-SEGs to ITK images in DICOMWeb datastore #1011

Merged
merged 7 commits into from
Sep 28, 2022

Conversation

jlvahldiek
Copy link
Contributor

Minor improvement to DICOMWeb datastore:

  • Now uses pydicom-seg in favor of dcmqi_tools's binary segimage2itkimage for conversion of DICOM-SEGs to ITK images
  • Now supports multiclass conversion

Copy link
Collaborator

@SachidanandAlle SachidanandAlle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this.. when I tried earlier version of pydicom it was not good enough.. let me test few samples and if all good then we can merge..

@pieper
Copy link
Contributor

pieper commented Sep 21, 2022

@jlvahldiek out of curiosity, what issues did you have with dcmqi that led you to make this switch? Also did you have a look at highdicom?

@jlvahldiek
Copy link
Contributor Author

@jlvahldiek out of curiosity, what issues did you have with dcmqi that led you to make this switch? Also did you have a look at highdicom?

I wanted to use MONAILabel's radiology app to train a UNET for pelvic segmentation (six segments; bone 1, bone 2, ...), Orthanc served as DICOMWeb datastore. Unfortunately, the current implementation only trained the last label and ignored all other labels. This led me to search for another solution.

pydicom-seg was already imported in monailabel/datastore/utils/convert.py, that's why I tried pydicom-seg first. Conversion results were very good for my use case (DICOM-SEGs of pelvic MRIs manually created with MONAILabel/Slicer/Orthanc). Have to admit that I did not test on other DICOM-SEGs.

I came across highdicom, might has some advantages. Not sure, should we switch to highdicom?

@pieper
Copy link
Contributor

pieper commented Sep 21, 2022

Thanks for the background @jlvahldiek, let's see what @SachidanandAlle finds when testing. For several of our projects (e.g. IDC) we are hoping to make use of the dicom seg format and want to be sure it's well integrated with MONAI. We have some experts available if there are questions about how to read/write the full range of supported segmentation objects.

@SachidanandAlle
Copy link
Collaborator

SachidanandAlle commented Sep 21, 2022

for DCO to pass.. you can use git commit -s option.. that should take care of DCO

And agree if we can reuse existing dependencies, it's great.. I did try highdicom before introducing dcmqi.. but it was in early stage of it's development.

I think for both pydicom and hidicom, the major problem I faced while converting mask to DICOM-SEG...
possibly the other way is fine (SEG to Mask)..

the current implementation only trained the last label and ignored all other labels
I guess this problem is not due to tool (dcmqi).. what it does it is writes multiple of them based on the segment number

--outputDirectory <std::string>
     Directory to store individual segments saved using the output format
     specified files. When specified, file names will contain prefix,
     followed by the segment number.

This needs to be corrected:
https://github.com/Project-MONAI/MONAILabel/blob/main/monailabel/datastore/utils/convert.py#L217

and then we can remove the TODO::
https://github.com/Project-MONAI/MONAILabel/blob/main/monailabel/datastore/utils/convert.py#L197

if there are multiple segs, then we load all of them and merge into non-overlapping (multi-channel) or overlapping (single channel with multi-class) based on the request/config/argument..

but if pydicom is doing the same.. and supporting this.. we can proceed with pydicom for this conversion.. i hope it's really reliable compared to dcmqi

@jlvahldiek
Copy link
Contributor Author

Okay, I see. Then I'm sorry, I didn't mean to cause confusion. There are good reasons to rely on dcmqi.

If I find time, I can try to correct https://github.com/Project-MONAI/MONAILabel/blob/main/monailabel/datastore/utils/convert.py#L217 in order to remove the Todo as @SachidanandAlle proposed.

Should I close this PR?

@fedorov
Copy link

fedorov commented Sep 24, 2022

I guess this problem is not due to tool (dcmqi).. what it does it is writes multiple of them based on the segment number

Yes, I understand this could be confusing. The reason it is doing that is because in the general case there may be overlapping segments, and saving one segment per file is the easiest way to make the output consistent independently of whether input contains overlapping segments or not.

@pieper
Copy link
Contributor

pieper commented Sep 25, 2022

The current generation of machine learning segmentation models are all based on labelmaps (non-overlapping segments) so giving dcmqi the option of performing the merge could be useful, perhaps generating a warning or error if overlap is detected. @fedorov

@SachidanandAlle
Copy link
Collaborator

SachidanandAlle commented Sep 25, 2022

We can have this conversion (overlap vs non-overlap/multi-channel label) based on the argument/config

@jlvahldiek
Copy link
Contributor Author

As far as I looked into pydicom-seg, their implementation takes care of overlapping/non-overlapping segments. Additionally, for non-overlapping segments one-hot encoding is applied and that's why this method may be used directly in MONAILabel's conversion method (as proposed in this PR).
I did not find something similar in dcmqi or highdicom. In case you like to stick to dcmqi at least some code is necessary to enable multi-class conversions.

By now I have successfully tested this PR's suggestion with 50+ pelvic MRIs and many more conversions (DICOM-SEG to nifty or nrrd). @SachidanandAlle, have you been able to do some testing?

@fedorov, is it possible to integrate merging/encoding into dcmqi's tool (as @pieper suggested)?

@SachidanandAlle
Copy link
Collaborator

Have asked @tangy5 and @diazandr3s to verify few corner cases for different modalities.. if all ok then, I have no problem to use pydicom.. we can merge this one

@lassoan
Copy link
Collaborator

lassoan commented Sep 27, 2022

pydicom-seg has a few serious limitations:

  • does not support writing of overlapping segments
  • does not seem to support any compression (RLE, deflate) for either reading or writing; maybe difficult to address (with good performance) without using C++

A minor limitation is that it depends on SimpleITK, which is a very large package and it is not ideal to depend on if someone already uses ITK via ITKPython (it should be easy to fix, though).

dcmqi supports read/write of overlapping segments and compression, but it is indeed inconvenient to use in Python (cannot be pip-installed, no native Python interface just CLI, cannot debug into it or develop in Python).

high-dicom does not have any of the limitations of pydicom-seg or dcmqi. If we switch from dcmqi then high-dicom seems to be a better choice than pydicom-seg.

@pieper
Copy link
Contributor

pieper commented Sep 27, 2022

Related to the highdicom option, it's already been integrated into MONAI Deploy and it would be nice to have symmetry in the MONAI Label code.

I spoke with Markus (@hackermd) and he says that highdicom should be able to directly handle this case well and offered that he, probably with @CPBridge, could draft a PR to help make this happen.

@jlvahldiek
Copy link
Contributor Author

Big thanks to all for the discussion and expert insights! For future projects, it will be very helpful for my team to convert DICOM-SEGs safely.

@SachidanandAlle
Copy link
Collaborator

OHIF/Viewers#2833

@tangy5
Copy link
Collaborator

tangy5 commented Sep 28, 2022

I've tested two tasks including a single label segmentation with spleen segs, and a multiple label segmentation of BTCV dataset (13 organs label). Currently there should be no issue, as I visualized the labels after dicom_seg_to_itk_image conversion. The pydicom-seg option is good upon the two test cases. For multiple segments, it's able to convert to one non-overlapping labelmaps.

If there are other options such as highdicom or simgle dicom-qi implementation, these tasks can be used as test cases.
Thanks for those discussion and insights, I will continue to prepare more test cases including MRI or other corner cases, will let us know if there are any issues.

@SachidanandAlle
Copy link
Collaborator

Related to the highdicom option, it's already been integrated into MONAI Deploy and it would be nice to have symmetry in the MONAI Label code.

I spoke with Markus (@hackermd) and he says that highdicom should be able to directly handle this case well and offered that he, probably with @CPBridge, could draft a PR to help make this happen.

Until this happens.. lets go with @jlvahldiek solution... if we encounter any critical issues/blocker we can revert.

@SachidanandAlle SachidanandAlle merged commit bb5367d into Project-MONAI:main Sep 28, 2022
@fedorov
Copy link

fedorov commented Sep 28, 2022

Sorry for not contributing to the discussion, I was traveling. I agree it makes a lot of sense to use highdicom over dcmqi in MONAILabel.

@SachidanandAlle
Copy link
Collaborator

Happy to see someone taking the initiative and add contributions on using/taking advantages of HIGHDicom in MONAILabel (I believe lots of things are added/enhanced and supported in current version of HighDicom.. a year ago I couldn't use it in monailabel for some limited features.. I even don't remember.. otherwise I would have always preferred library over binaries for these conversion utility)

Douwe-Spaanderman pushed a commit to Douwe-Spaanderman/MONAILabel that referenced this pull request Dec 9, 2022
…roject-MONAI#1011)

* renaming

* change dicom-seg to itk image conversion

Signed-off-by: Janis Vahldiek <[email protected]>

* remove unnecessary binaries

Signed-off-by: Janis Vahldiek <[email protected]>

* make flake8 happy

Signed-off-by: Janis <[email protected]>

Signed-off-by: Janis Vahldiek <[email protected]>
Signed-off-by: Janis <[email protected]>
Co-authored-by: Andres Diaz-Pinto <[email protected]>
Co-authored-by: tangy5 <[email protected]>
Co-authored-by: SACHIDANAND ALLE <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants