Review ClearCanvas plugin terminology for reuse

[fedorov]

See

Segmentation/Configuration/AnatomicRegionAndModifier.xml Segmentation/Configuration/SegmentationCategoryTypeModifier.xml

in https://github.com/fedorov/ClearCanvas-AimPlugin4.5

[fedorov]

https://github.com/fedorov/ClearCanvas-AimPlugin4.5/blob/master/Segmentation/Configuration/AnatomicRegionAndModifier.xml https://github.com/fedorov/ClearCanvas-AimPlugin4.5/blob/master/Segmentation/Configuration/SegmentationCategoryTypeModifier.xml

[fedorov]

To clarify this issue, anatomic region and segmentation category need to be populated in DICOM SEG, see here as an example: https://github.com/QIICR/Iowa2DICOM/blob/master/ConvertSegmentations/EncodeSEG.cxx#L346-L352

The idea is to have a free text dynamic search based on code meaning for the the region and modifier, and for category and its modifier. It is a quite long list, and I don't know how dynamic search is implemented in Qt or other tools we have. That needs to be investigated.

Let me know if we need to discuss more.

[naucoin]

The Models module has a "scroll to" search box, and the color picker pop up also has a search box, so there are some examples I can follow.

[fedorov]

The module search box in the main toolbar is also a good one.

[naucoin]

@fedorov: after our meeting today, my understanding of the phases of this change are:

1. Add information (non-editable) to the Editor Module under the color picker Label widgets that show the color name, value and color to show the terminology that has been looked up: Category: [ ] Modifier: [ ] Region: [ ] Modifier: [ ] When the user picks a color, the informatin is updated, but the boxes won't show if the color table isn't linked to terminology (or should we show generic values?). Add this information to the label map via node attributes.
2. Allow the user to edit the Category and Region by clicking on them to search a list of terms. This will allow resetting from the generic values.
3. If a user has chosen a color table that doesn't have the terminology mapping defined, create a way to edit them easily for each value (add columns to the Colors module editor? workflow that gives you one label at a time?). Save the new terminology mapping with the segmentation object.

To keep in mind:

• terminology may be on a remote server
• further integration of the terminology widget across Slicer
• coordinate with the Queen's group on the new segmentation and editor work

[fedorov]

Yes, this is good. Few clarifications:

• connection between color LUT and terminology is currently maintained in a separate LUT, which at the moment is defined by a file in Reporting just for GeneralAnatomy
• if a given color LUT has a terminology LUT, then correspondence between label ID/color and the term is fixed, but otherwise user should be able to define correspondence as needed
• yes, if there is no terminology assigned, it should default to some generic term
• for number 3, will review down the road to decide on the plan ...

[naucoin]

For clarification, in the Reporting implementation there's a structure with: SegmentedPropertyCategory SegmentedPropertyType SegmentedPropertyTypeModifier

Can you explicity map out the correspondence for me between these values and the Category, Category Modifier? I'm also assuming you want to see the CodeMeaning rather than the Value or SchemeDesignator?

[fedorov]

Can you explicity map out the correspondence for me between these values and the Category, Category Modifier?

It seems to me that SegmentationCategoryTypeModifier.xml alone will be enough to map something like Slicer GenericAnatomyColors (http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/Reporting). You have a list of categories, then depending on a category you initialize type selector, and depending on category/type, you initialize the modifier selector.

AnatomicRegion + modifier are optional. So the way I see it is to have a separate selector with all anatomic regions, and a selector with modifiers as needed based on the selected region.

I'm also assuming you want to see the CodeMeaning rather than the Value or SchemeDesignator?

Yes, absolutely. At least when the search is done, it should be by the text in the meaning. When a code is selected, it probably makes sense to communicate the triple to the user as a tooltip, or as a static text underneath the selectors.

[naucoin]

@fedorov I refactored the color logic[1] to provide an API to create new terminology mappings for color nodes. The commit message includes a python example, calling AddTermToTerminology will create a new mapping if it doesn't exist for the given LUT and then create the structures needed to add the passed strings to the terminology. I'm still considering breaking it down more (like allowing adding just the category by itself, then the type, then the modifier), let me know what you think about this level of detail.

[1] https://github.com/naucoin/Slicer/commit/d168a245131eba90f6b6baa2dda387c618cfc3e6

[fedorov]

@naucoin here is the branch of Reporting updated to read SEG objects with multiple segments: https://github.com/fedorov/Reporting/tree/refactor-to-new-dcmtk

The specific location you need to update to hook up color and terminology are here: https://github.com/fedorov/Reporting/blob/refactor-to-new-dcmtk/Py/DICOMSegmentationPlugin.py#L124-L130

Let's discuss sometime soon. I think it should be relatively straightforward to finish this up and have multi-segment SEGs reading integrated. Currently, each segment is loaded into a separate label node, but I think I might later implement a check that takes all segments that do not overlap and merges them into one label node. Need to think what is the right approach.

In a related development, I talked with @pieper today, and he is looking into integration of writing multi-segment SEGs using the same approach used for reading (the groundwork is done by a CLI, and python for integration). For writing, the CLI to be used as the basis is here: https://github.com/QIICR/Iowa2DICOM/blob/master/ConvertSegmentations/EncodeSEG.cxx

[naucoin]

Looks straight forward. Do you have a sample .info file?

[fedorov]

The text in the comment in the code is literally cut and paste from the info file.

I will send you a sample SEG so you can generate one.

[fedorov]

Nicole, I sent you a sample SEG with 10 segments by email. With the branch I referenced, you should be able to import it into the DICOM browser, and load all 10 segments as separate labels.

[naucoin]

I built your branch and tried to load the tumor_User1_Manual_Trial1.dcm file, working through a few issues in the code (it crashed because I usually start without CLIs loaded, debugging a labelName missing issue).

[fedorov]

You cannot start without CLIs loaded, the plugin is dependent on the SEG2NRRD CLI bundled in the extension.

[naucoin]

It shouldn't crash though, so I'm putting in a warning.

[fedorov]

FYI here's how "SEG browsing" interface looks like in ClearCanvasAIM

[fedorov]

As a side note, I have to admit I like a lot how units are natively connected with the measurements in ClearCanvas:

[naucoin]

SEG2NRRD uses the voxel value 1 for all segmentations, so the color mapping isn't working correctly. Any objections to me changing it to use the segmentId or does it need to be filled with just 0 and 1 values? https://github.com/fedorov/Reporting/blob/refactor-to-new-dcmtk/SEGSupport/SEG2NRRD.cxx#L340

[naucoin]

Setting the color table color names to the full terminology is not working well with the data probe, the screen capture shows how it was expanded after I moused over the segment.

[naucoin]

Updated editor with Region

[fedorov]

OK to assign segment number in place of 1.

Yes, the names are too long. I think the best alternative is to have a specialized term browser. But I think this is not immediate priority. For now, maybe you can look into setting full term names as tooltips for the colors table rows?

[naucoin]

I know I could do a right click on the color table rows to pop up terms, I'll have to look into tool tips. Another option would be to have the same set up as in the editor where when you click on a color in the colors module, it updates the terminology widgets. I'd probably translate the editor scripted panel into a c++ widget that can be more easily reused in that case though.

[fedorov]

Table items tooltips: http://doc.qt.io/qt-4.8/qtablewidgetitem.html#setToolTip

[fedorov]

I would also prefer single column for displaying the items, and as discussed, full terminology concatenation should not show up in the DataProbe.

[naucoin]

Tool tips in the Colors module: https://github.com/naucoin/Slicer/commit/5090979853da5c8be5fd1721edac2e1593b64f58

[naucoin]

@fedorov @pieper Update and rebased against this evening's Slicer trunk, with renamed methods, some new utility methods and a minor bug fix that should avoid the issue Andrey mentioned on the hangout: https://github.com/naucoin/Slicer/commit/5f6ef29d1e46647fb1dda7eabe6671c602ef845a Topic branch: https://github.com/naucoin/Slicer/tree/4047-Add-semantic-color-information-to-color-selector-in-editor

Reporting update to use the new methods and trying to update the self test, but @pieper I wasn't able to quite figure out which terms you needed, I put all of them there so you can use them (nb: the code meaning, value, scheme may not be in the right order from the concatenated util methods that I added). https://github.com/naucoin/Reporting/commit/4b15dbc354ce8015292f438c612dfa85f6d8a03d I can do a pull request but the self test that Steve wrote isn't quite working yet.

[pieper]

The changes look great! Some smallish things...

In the comment you have CodeValue,CodeMeaning,CodingSchemeDesignator, but in the info file I believe it needs to be CodeValue,CodingSchemeDesignator,CodeMeaning. Is that the order thing you meant?

Here's where they are parsed:

https://github.com/naucoin/Reporting/blob/master/SEGSupport/SegmentAttributes.h#L233-L238

So they should not be in the order listed in the comment (either the comment needs to change or the method or both).

Also as you have the self test now it implies that propertyType can be optional but Andrey's code says it is required.

https://github.com/naucoin/Reporting/blob/master/SEGSupport/EncodeSEG.cxx#L339-L345

Also, if something like SegmentedProperytCategory aren't defined in the terminology won't the color logic return an empty string? If so then the calls like propertyCategoryWithColons.replace(':',',') will cause and exception.

A more big picture question which you guys may have thought through already: what shall we do when the terminology does not have required fields like SegmentedPropertyType? Do we know if the standard has a designated code for "unknown property type"? The same question applies to all the other required values. Maybe this is a question for David.

On Wed, Nov 11, 2015 at 6:42 PM, Nicole Aucoin notifications@github.com wrote:

@fedorov https://github.com/fedorov @pieper https://github.com/pieper Update and rebased against this evening's Slicer trunk, with renamed methods, some new utility methods and a minor bug fix that should avoid the issue Andrey mentioned on the hangout: naucoin/Slicer@5f6ef29 https://github.com/naucoin/Slicer/commit/5f6ef29d1e46647fb1dda7eabe6671c602ef845a Topic branch:

https://github.com/naucoin/Slicer/tree/4047-Add-semantic-color-information-to-color-selector-in-editor

Reporting update to use the new methods and trying to update the self test, but @pieper https://github.com/pieper I wasn't able to quite figure out which terms you needed, I put all of them there so you can use them (nb: the code meaning, value, scheme may not be in the right order from the concatenated util methods that I added). naucoin@4b15dbc https://github.com/naucoin/Reporting/commit/4b15dbc354ce8015292f438c612dfa85f6d8a03d I can do a pull request but the self test that Steve wrote isn't quite working yet.

— Reply to this email directly or view it on GitHub https://github.com/fedorov/Reporting/issues/52#issuecomment-155945752.

[naucoin]

@pieper I double checked the .info file parsing in DICOMSegmentationPlugin.py and yes, it produced .info files of code value, coding scheme designator, code meaning as well, I'll fix the order of the returned triple string. I'll also reorder things in the big AddTermToTerminology call as well as the data structure for clarity.

In SEGExporterSelfTest code, I hadn't figured out exactly what you were trying to output there since there were some debugging style statements but no comment with the intent (late in the day coding, I saw that the extension was .txt rather than .info and didn't want to assume you were writring the same format as the .info files I get from the segmentation plugin[1]). If you want an error condition added for the non optional ones I can do that, and I'll take another look at the code to try and match the format of the plugin.

I double checked that string.replace() on an empty string wasn't causing an exception in the python console, but I can add empty string checks first.

[1] https://github.com/naucoin/Reporting/blob/master/Py/DICOMSegmentationPlugin.py#L147

[fedorov]

A more big picture question which you guys may have thought through already: what shall we do when the terminology does not have required fields like SegmentedPropertyType? Do we know if the standard has a designated code for "unknown property type"?

Let's bring the power of github here - including @dclunie!! :)

As a background for David - Slicer color tables were originally used to establish linkage between the segmentation pixel values, RGB color and essentially free-text label. GeneralAnatomy is one such lookup table that as you may remember we annotated with your help using SNOMED terminology (wiki.slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/Reporting). Now we are working to integrate terminology more tightly into Slicer with the SEG export being the main driving use case.

The way I thought about the Slicer SEG export and terminology related issues is that we have codes for GeneralAnatomy, and for the color tables that were created dynamically while importing SEGs. For other color tables, we would put a very generic term like "Tissue" for every label. I think "Tissue" would be semantically correct for most cases, although imprecise. Of course, it will not cover cases like foreign objects, table, phantom segmentations etc.

I searched just now, and there is code (111176,DCM,Unspecified) in CID 6040 "Non-lesion object types". @dclunie - would it pass your threshold if we use this code for segmentation object segments where the user did not specify the term, or we cannot determine it from predefined mappings? I personally think this would be way better than what some other workstations do. For example, segmentations done in syngo.via seem to populate anatomical information from the BodyType field of the source image, so whatever I segment in a chest CT will be assigned Chest code.

[naucoin]

Looks like the DICOMSegmentationPlugin that I was starting from uses a slightly different format (from SEG2NRRD) than EncodeSEG.

DICOMSegmentationPlugin: file named label #.info, formatted with line breaks, one colon, comma separted: RGBColor:128,174,128 AnatomicRegion:T-C5300,SRT,pharyngeal tonsil (adenoid) AnatomicRegionModifier:code,scheme,meaning SegmentedPropertyCategory:M-01000,SRT,Morphologically Altered Structure SegmentedPropertyType:M-80003,SRT,Neoplasm, Primary SegmentedPropertyTypeModifier:code,scheme,meaning

EncodeSEG is expecting a list of files with a line per label, semi colon separated instead of line breaks (current broken example that I'm trying to fix): 1;T-D0050,SRT,Tissue;SegmentedPropertyType:T-D0050,SRT,Tissue;SegmentAlgorithmType:AUTOMATIC;SegmentAlgorithmName:SlicerSelfTest

Do we want to keep supporting both?

[fedorov]

Do we want to keep supporting both?

We can switch SEG2NRRD to use the format used by EncodeSEG. If you look at EncodeSEG, it has code to parse those files that perhaps can be reused in Slicer. The reason EncodeSEG has that format is because inputs can have multiple labels. I was thinking to augment SEG2NRRD later with a capability to merge segments, so it will be handy as welll. We can do this later -- the format used by EncodeSEG is not parsed by Slicer, so I think not much support is needed beyond writing out semicolon separated strings.

[naucoin]

This version of the .txt file from EncodeSEG lets the self test pass: 1;SegmentedPropertyCategory:T-D0050,SRT,Tissue;SegmentedPropertyType:T-D0050,SRT,Tissue;SegmentAlgorithmType:AUTOMATIC;SegmentAlgorithmName:SlicerSelfTest Checked in as: https://github.com/naucoin/Reporting/commit/77e89f0af243d587d3a5043b05b18b800a71148d Relying on: https://github.com/naucoin/Slicer/commit/ab484bf4e65894a42d48faea8cf3f54ada5e7de4 @fedorov @pieper : needs to be updated to sort out the file format and what to do in case of missing required fields.

[naucoin]

Slicer pull request: https://github.com/Slicer/Slicer/pull/408

[fedorov]

@pieper things seem to work fine, so the only issue remaining is the support of export at the user level from the subject hierarchy.

[pieper]

Great - now we're down to just the interface issues. Putting it in the Subject Hierarchy makes sense, but it might also be good to have a shortcut in the Editor. Let's make a concrete plan on Monday afternoon.

On Fri, Nov 13, 2015 at 3:59 PM, Andrey Fedorov notifications@github.com wrote:

@pieper https://github.com/pieper things seem to work fine, so the only issue remaining is the support of export at the user level from the subject hierarchy.

— Reply to this email directly or view it on GitHub https://github.com/fedorov/Reporting/issues/52#issuecomment-156554970.

[naucoin]

Infrastructure changes commited to Slicer: https://github.com/Slicer/Slicer/commit/1f0a534f1979e008a371d6873438633be58ae227 http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24742

[fedorov]

This issue is addressed separately by handling terminology directly in the new Segmentation Editor. This is under development by @cpinter