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

handle fractional SEG objects #1346

Open
pieper opened this issue Jan 3, 2020 · 14 comments · May be fixed by dcmjs-org/dcmjs#309
Open

handle fractional SEG objects #1346

pieper opened this issue Jan 3, 2020 · 14 comments · May be fixed by dcmjs-org/dcmjs#309
Assignees
Labels
IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority v2-legacy OHIF v2

Comments

@pieper
Copy link
Member

pieper commented Jan 3, 2020

Request

Related to #1345, the segmentation is encoded as fractional but we currently don't load it.

Instead we throw this as an error:

Uncaught (in promise) Error: Fractional segmentations are not supported

What feature or change would you like to see made?

We should probably just threshold this to binary for now. Better of course would be to handle the fractional case as a kind of heatmap overlay.

Why should we prioritize this feature?
It's part of the DICOM standard and we have examples of data encoded this way.

@pieper pieper added Community: Request ✋ IDC:priority Items that the Imaging Data Commons wants to help sponsor labels Jan 3, 2020
@JamesAPetts
Copy link
Member

JamesAPetts commented Jan 6, 2020

Better of course would be to handle the fractional case as a kind of heatmap overlay.

Yeah this one is interesting, this would be the best thing to do, but it'd essentially be a different data type in cornerstoneTools. I think the simplest thing to do would be to save each segment as its own labelmap as per standard and give it its own colorLUT that is the same color with different alpha per value?

Is it common to have multiple fractional segments in one DICOM SEG on one referenced series?

@pieper
Copy link
Member Author

pieper commented Jan 6, 2020

Is it common to have multiple fractional segments in one DICOM SEG on one referenced series?

Not sure if there's any consensus on this, but if it's okay according to the standard then probably somebody will do it. It would be great if we could work on promoting best practices for how people should structure their SEG instances. (Comments @fedorov ?)

@JamesAPetts
Copy link
Member

I'm sure its DICOM compliant, but the web dev in me is already screaming internally about the potential consequences on resource allocation :).

@fedorov
Copy link
Member

fedorov commented Jan 6, 2020

From a quick look, I am not sure how this is related to #1345 - can you clarify? Also, considering we don't have any fractional SEGs in TCGA, and it's not in IDC MVP, I don't consider this to be IDC priority.

I have very limited experience with fractional SEG, I don't think I have seen any examples other than the objects produced by BrainLab, and I don't know of any tools that create fractional SEGs.

@fedorov fedorov added IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority and removed IDC:priority Items that the Imaging Data Commons wants to help sponsor labels Jan 6, 2020
@fedorov
Copy link
Member

fedorov commented Jan 6, 2020

We confirmed with @pieper that the segmentations in that kidney collections are binary, so the question is why that fractional error is coming up.

@fedorov
Copy link
Member

fedorov commented Sep 12, 2022

It turns out we do have fractional segmentations in IDC, which I think turns this supporting fractional segmentations into a priority. @Punzo how difficult is this going to be to implement?

Example dataset from IDC: https://viewer.imaging.datacommons.cancer.gov/viewer/1.3.6.1.4.1.14519.5.2.1.7695.4164.330974290504454152904316943429

@fedorov fedorov added IDC:priority Items that the Imaging Data Commons wants to help sponsor and removed IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority labels Sep 12, 2022
@Punzo
Copy link
Contributor

Punzo commented Sep 13, 2022

@fedorov supporting fractional segmentations is rather complicated and it would involve various steps:

  1. for cornerstone2D we need to modify the segmentation tools infrastructure to allow rendering each segment with a LUT. This will be very heavy in resources as James already pointed out: e.g. for each segment we would need to allocate its own label map (and give it its own colorLUT that is the same color with different alpha per value);
  2. In dmcjs/OHIF load the fractional segmentation with the new cornerstone tools infrastructure.
  3. apply the same concept for VTK extension. Here comes an issue: since in OHIFv2 we support only one labelMap per segmentation. We had already this issue with overlapping binary segmentations. In this last case, we decided to skip the support for overlapping binary segmentations in the VTK extension for OHIFv2 (the main issue is memory, since in the vtk extension you need to multiply by 3 the memory usage);
  4. revisit UI (e.g. settings have been designed specifically for binary label maps);
  5. probably makes sense to visualize only one series at time (while for binary label maps we allow rendering from different series, i.e. active/inactive label maps).

@pieper
Copy link
Member Author

pieper commented Sep 13, 2022

I believe handling fractional segmentation is going to be much easier in OHIF v3 (modulo resource issues). Maybe for v2 we could just binarize the segmentations (with a user warning).

@fedorov
Copy link
Member

fedorov commented Sep 13, 2022

Maybe for v2 we could just binarize the segmentations (with a user warning).

This might make sense. It might be that all fractional segs are actually binary, but saved as fractional. We should to check.

@Punzo
Copy link
Contributor

Punzo commented Sep 13, 2022

Ok I will check the actual values in the array. In case, should I use a simple thresholding for binarizing them? I fear anything more convoluted can be slow in JavaScript.

@pieper
Copy link
Member Author

pieper commented Sep 13, 2022

yes, just thresholding should be fine. Probably just zero/non-zero but let us know if that doesn't look right for the data values.

@Punzo
Copy link
Contributor

Punzo commented Sep 14, 2022

PR with summary of my findings is here: dcmjs-org/dcmjs#309 @pieper @fedorov

@Punzo Punzo assigned GitanjaliChhetri and unassigned Punzo Sep 29, 2022
@georg-walther
Copy link

I would really appreciate OHIF's support for fractional segmentations. I have heatmaps formatted as DICOM-SEG. @fedorov There are tools to create fractional SEGs: https://github.com/razorx89/pydicom-seg/tree/feature/write-fractional

@fedorov
Copy link
Member

fedorov commented May 5, 2023

@georg-walther the issue is that (from my perspective, as IDC https://portal.imaging.datacommons.cancer.gov/ lead), we do not have any fractional SEG series in IDC, while at the same time we have a lot of other issues in OHIF that are in the critical path for IDC, so I cannot prioritize working on this issue.

Can you at least share a sample fractional SEG and the accompanying image series, and attach to this issue, so this can be worked on in the future?

@fedorov fedorov added IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority and removed IDC:priority Items that the Imaging Data Commons wants to help sponsor labels May 5, 2023
@igoroctaviano igoroctaviano added the v2-legacy OHIF v2 label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDC:candidate Possible feature requests for discussion, and if we agree, they can be relabeled IDC:priority v2-legacy OHIF v2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants