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

Work in progress to add LABELMAP support #491

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

spalte
Copy link
Collaborator

@spalte spalte commented Jan 30, 2024

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.

@spalte spalte force-pushed the spalte_labelmaps branch 3 times, most recently from 97a1abf to ead47cf Compare January 30, 2024 01:48
@spalte spalte marked this pull request as draft January 30, 2024 01:50
@michaelonken
Copy link
Member

michaelonken commented Jan 30, 2024

@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
Copy link
Member

fedorov commented Jan 30, 2024

@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
Copy link

It is not the intent to limit labelmap segs to 3D. See detailed reply here ImagingDataCommons/highdicom#234 (comment)

@CPBridge
Copy link

@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
Copy link
Member

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

@dclunie
Copy link
Member

dclunie commented Jan 31, 2024

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
Copy link
Member

dclunie commented Jan 31, 2024

@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
Copy link

CPBridge commented Feb 1, 2024

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

@michaelonken
Copy link
Member

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

Great, thanks! :-)

Copy link

sonarcloud bot commented Nov 20, 2024

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.

5 participants