QIICR / dcmqi

dcmqi (DICOM for Quantitative Imaging) is a free, open source C++ library for conversion between imaging research formats and the standard DICOM representation for image analysis results
https://qiicr.gitbook.io/dcmqi-guide/
BSD 3-Clause "New" or "Revised" License
240 stars 61 forks source link

Work in progress to add LABELMAP support #491

Open spalte opened 9 months ago

spalte commented 9 months ago

This pull request to meant to assist in sharing the current work in progress for adding LABELMAP support.

16 bit label map support still needs to be added. The behavior with 16 bit label maps is undefined as is.

michaelonken commented 9 months ago

@fedorov : We had an interesting discussion this morning which has been triggered by the work done by @spalte last night. I checked the pull request and realized he was working on the overlap detection code I recently added, and the labelmap supplement clearly forbids overlaps -- so there is no need for checking probably we don't need the overlap checking for the labelmap case.

However, Joel correctly stated that it is still possible to create overlapping labelmaps by just putting different (DICOM) frames into the same position that project multiple segments onto the same pixel in space. I.e. what might be forbidden by the standard is not inherently enforced by the data model which was counterintuitive to (at least my) brain's model of labelmaps so even the possibility did not came to my mind at all . [1]

So we asked @dclunie about his view on this and @pieper and @CPBridge joined the discussion. We were agreeing that there should be extra guidance in the supplement that such kind of overlaps must be avoided.

An interesting point has been raised by Chris in this context: If one has cine runs, i.e. frames recorded at different points in time, a segmentation might well contain frames with the same frame position at different time point points (which is not forbidden right now), it even makes sense to encode things this way. The question is whether this is allowed (as I understood, it is right now), whether it should be allowed (and then must be encoded correctly using the Segmentation's dimension setup), and so on.

Then the discussion moved on what needs to be considered when you have the same and/or different label maps spread over multiple files and how to keep track of segments. But that is another topic.

We got interrupted by a Slicer session but I think David decided to sit down and think/write a proposal how to handle all this.

@ everybody interested: please don't hesitate to add/fix if I forgot or misunderstood something.

[1] Actually, this scenario also applies to the existing binary and fractional segmentation objects (multiple frames at the same position can "overwrite" or "extend" each other's segment pixel coverage). I think our current code in dcmqi does not crash or do something stupid in those cases, but maybe we could spit out some warnings or so if we can do it "en passant".

fedorov commented 9 months ago

@michaelonken thank you for this summary!

Related to the issues you raised, my question is whether the new LABELMAP object is intended to be restricted to 3 dimensions? If there is a frame that occupies the same space when projected from 4 to 3 dimensions, is this supposed to be a violation of the standard?

I do not see why it should be, and it is an interesting point, since this would mean LABELMAP becomes a more general purpose representation.

CPBridge commented 9 months ago

It is not the intent to limit labelmap segs to 3D. See detailed reply here https://github.com/ImagingDataCommons/highdicom/pull/234#issuecomment-1917956906

CPBridge commented 9 months ago

@spalte @michaelonken Here is an example labelmap seg created with the highdicom implementation. Happy to create others with different options if that would help.

https://cloud.chrisbridge.science/s/xHsY7bDeYd9KW9s

michaelonken commented 9 months ago

@spalte This this DCMTK repo (Labelmap branch) for DCMTK WIP for this.

dclunie commented 9 months ago

I notice there is no SegmentSequence entry for an index value of 0 (background) (SegmentNumber 0, not allowed other than in type LABELMAP in proposal) - would it would be worth creating one of these (optionally perhaps)?

@spalte @michaelonken Here is an example labelmap seg created with the highdicom implementation.

dclunie commented 9 months ago

@CPBridge I created a new version of dciodvfy that handles the draft Sup 243 changes so far and uploaded a MacOS executable to DropBox here - use the '-profile Sup243' command line option to activate the validation of the draft changes; it assumes the ordinary Segmentation Storage SOP Class that you are using in your examples so far (which will not be the case in the long term, but until we have one assigned will suffice).

CPBridge commented 9 months ago

@michaelonken here are some more examples including ones using palette colour: https://cloud.chrisbridge.science/s/3AaQACM9tBobkNm

michaelonken commented 9 months ago

@michaelonken here are some more examples including ones using palette colour: https://cloud.chrisbridge.science/s/3AaQACM9tBobkNm

Great, thanks! :-)

sonarcloud[bot] commented 2 days ago

Quality Gate Passed Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud