ImagingDataCommons / highdicom

High-level DICOM abstractions for the Python programming language
https://highdicom.readthedocs.io
MIT License
172 stars 37 forks source link

Add "labelmap" segmentations #234

Closed CPBridge closed 2 days ago

CPBridge commented 1 year ago

This is an experimental feature for NA-MIC project week 2023

CPBridge commented 1 year ago

Update on the status of this. I successfully implemented "LABELMAP" style segmentations using the existing highdicom.seg.Segmentation class __init__ method at project week. You can trigger this by using segmentation_type="LABELMAP". This should work with all the input array formats currently supported by BINARY segmentations (except overlapping segments stacked down the final dimension).

Currently, the get_pixels... methods will not work correctly with "LABELMAP" style segmentations, I will try to add this at some point for prototyping purposes.

David Clunie is currently drafting up a formal proposal of what this would look like. I will try to keep this branch up to date with the formal proposal. Notably, there will likely be a new IOD created, rather than using the existing IOD with a new SegmentationType, and this will involve some refactoring of the codebase.

CPBridge commented 8 months ago

I have rebased this branch on the current release (0.22.0) so that it is now possible to use features from that release in combination with labelmaps, e.g. TILED_FULL segmentations, autotiling, multi-threaded encoding.

The plan is still to keep this as a draft pull request until the labelmap segmentation becomes part of the standard (assuming it does). However it remains a fully functional reference implementation for creating (but not yet parsing) labelmap segmentations in line with the current proposal.

fedorov commented 8 months ago

@CPBridge related to the discussion in https://github.com/QIICR/dcmqi/pull/491#issuecomment-1916564778, are you restricting the dimensionality of LABELMAP in any way?

If I understand correctly, there is nothing that would prevent it from being n-dimensional, which would arguably allow to completely supersede binary SEG! Am I missing something?

CPBridge commented 8 months ago

@fedorov the labelmap implementation on this branch is a generalisation of the BINARY/FRACTIONAL implementation in the latest release. Consequently it inherits all options and constraints from that implementation, except where those options/constraints explicitly differ between the BINARY and LABELMAP (e.g. segments may not overlap).

However this implementation is not fully general it what it allows users to create. Currently there are three options:

Support for other (or custom) indexing is something I have considered adding and probably will at some point, but it is pretty complex.

In our conversation today I mentioned the 2D+T cine case merely theoretically, because it could in principle be observed.

The point of some disagreement boiled down to whether we should allow people to define two different labelmaps at the same image position. One reason for this night be to allow for multiple overlapping groups of segments.

@pieper @dclunie @spalte @michaelonken

michaelonken commented 7 months ago

@CPBridge Hey Chris, do you have 8 and 16 bit PALETTE COLOR examples available as produced by highdicom? I am returning to the labelmap work in DCMTK/dcmqi. Thanks!

CPBridge commented 7 months ago

@michaelonken there are several different versions in this shared folder. Note that the color palettes were improvised quickly and not very sensible, but they are valid.

CPBridge commented 4 months ago

Pasting the latest version of the DICOM supplementĀ here: https://dicom.nema.org/medical/dicom/Supps/LB/sup243_lb_LabelMapSeg.pdf

CPBridge commented 4 months ago

Some to-dos upon re-reading the latest supplement:

Further thoughts:

fedorov commented 1 week ago

@CPBridge - @dclunie is reminding me that this is now in the final text, and now this PR can be completed, whenever you have cycles!

CPBridge commented 1 week ago

Indeed, it's on the radar. Unfortunately this may take a while. In the meantime, on the volumetric branch (#277) I am significantly refactoring the segmentation code to have a shared underlying MultiframeImage class so that we don't have to have a load of shared code between segs, labelmaps, parametric maps etc and don't have to solve the same problems (volumes, tiling, efficiency) over and over again. Then once that's in place, it should be relatively straightforward to tweak this implementation to just be a special case of the general implmentation

michaelonken commented 1 week ago

I also have it on radar for DCMTK but the feature needs to wait a bit. It requires some extra code e.g. for handling of padding value but also major merging and testing and the time I can spend on this right now is limited as well.

fedorov commented 1 week ago

Thank you both! šŸ‘

CPBridge commented 2 days ago

Merging this branch for creation of segmentations. A further branch will handle reading before the next release