Replies: 5 comments 3 replies
-
@CPBridge I implemented the enhancement. Tried to be as generic as possible. Would you like to review it? |
Beta Was this translation helpful? Give feedback.
-
Yes of course
…On Sun, Jan 12, 2025, 10:13 AM kavmar ***@***.***> wrote:
@CPBridge <https://github.com/CPBridge> I implemented the enhancement.
Tried to be as generic as possible. Would you like to review it?
—
Reply to this email directly, view it on GitHub
<#512 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5PFLFQNC6D3KTCLO7S3HL2KKWDVAVCNFSM6AAAAABUWBI4DWVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTCOBRGMZDSMI>
.
You are receiving this because you are subscribed to this thread.Message
ID:
<Project-MONAI/monai-deploy-app-sdk/repo-discussions/512/comments/11813291
@github.com>
|
Beta Was this translation helpful? Give feedback.
-
Here you are. |
Beta Was this translation helpful? Give feedback.
-
Hi @kavmar I think the problem here is not exactly what you think that it is. It is true that highdicom requires that the segment descriptions it is passed to be consecutive, starting at 1. However in the DICOMSegmentationWriterOperator this should be handled already by this line which automatically numbers the segments for you according to their index in the list you pass. However highdicom does not require that every described segment be present. It is perfectly acceptable for you to specify that "tumor" is class 2, and then pass an array that contains no 2s. In fact, this is very important because then the receiver can understand that there is no tumor present (as opposed to the presence of tumor not being assessed). However the segmentation operator was written to assume that the values in the input image passed to it exist in the range 0 (background) to len(segment_descriptions) (inclusive). I.e. if you pass five segment_descriptions to the DICOMSegmentationWriterOperator constructor, you should pass it an image that contains only the values {0, 1, 2, 3, 4, 5} (although not all of those values need to be present, as explained above). So you could pass an array that contains {0, 1, 3, 4} but you could not pass an array that contains {0, 1, 3, 7}. This seemed a reasonable assumption at the time given that nearly all segmentation models perform this renumbering anyway due to the need to move to a one-hot representation of the classes in order to be differentiable. This assumption should have been better documented and/or checked for, however. If instead you provide an array containing values outside this range, highdicom will complain at you as you have found. In order to better suggest how to address this, it would really help to understand how you came to have a segmentation mask with non-consecutive values, given that this is pretty uncommon in ML models. How does your model architecture allow for this? Maybe we should add an optional argument to the SegmentDescription class. I.e. you could tell the operator via each segment's description how it will be encoded in the input array. If None, it will be inferred from the position in the segment_descriptions list, as is the current behaviour. I am happy to review a change, but would it be possible to have it as a pull request on github? That would be much easier to review, comment on, discuss, etc? |
Beta Was this translation helpful? Give feedback.
-
Hi @CPBridge I created the pull request. #517 (comment) Please kindly review if it makes sense. Really, thanks a lot for in-depth explanation. I am working on an monai-deploy app based on https://github.com/Project-MONAI/VISTA/tree/main/vista3d All the classes are coded in https://github.com/Project-MONAI/VISTA/blob/f16b175daa38a2894555e17053d240ebaa9a6501/vista3d/data/jsons/label_dict.json Thanks |
Beta Was this translation helpful? Give feedback.
-
Hi,
I am working on a MAP similar to examples/apps/ai_unetr_seg_app/app.py - different model, but want to output DICOM SEG objects. My model infers labels of organs from CT images, which do not have contiguous numbers, e.g. 1, 3, 4, 5, 7, 8 ...
There are two similar situation, when labels could be non-contiguous:
a) it could also happen that a label is missing or has not been detected by algorithm at all for one reason or another, such as missing kidney in ai_unet_seg_app, or
b) a model can infer classes from CT thorax-abdomen-pelvis, but only CT abdomen-pelvis is used as an input
These two situations are equivalent, AFAIK. However, lines 104 - 118 of the app.py
monai-deploy-app-sdk/examples/apps/ai_unetr_seg_app/app.py
Line 104 in 9608cd7
Issue is that according to DICOM SEG standard the segment numbers must start from 1 and be contiguous with increment 1. However, at the time of application/DICOMSegmentationWriterOperator compose it is not known, which classes will or will not be inferred. Hence, I believe that DICOMSegmentationWriterOperator should internally consolidate SegmentDescriptions based on runtime inferred classes, before create_dicom_seg creates DICOM SEG result:
monai-deploy-app-sdk/monai/deploy/operators/dicom_seg_writer_operator.py
Line 318 in 9608cd7
highdicom is checking contiguity of numpy array/Image with labels, but stops, if these are not contiguous.
@CPBridge as discussed in highdicom, what are your thoughts as the author of the DICOMSegmentationWriterOperator
Beta Was this translation helpful? Give feedback.
All reactions