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

Filter duplicate measurements instead of disabling plugins #209

Open
fedorov opened this issue Nov 24, 2017 · 9 comments
Open

Filter duplicate measurements instead of disabling plugins #209

fedorov opened this issue Nov 24, 2017 · 9 comments
Assignees

Comments

@fedorov
Copy link
Member

fedorov commented Nov 24, 2017

Follow up on the discussion in #207 (comment)

@che85 che85 self-assigned this Dec 4, 2017
@che85 che85 added this to the NAMIC-Winter2018 milestone Jan 3, 2018
@che85
Copy link
Member

che85 commented Jan 11, 2018

@chribaue after talking with @fedorov about this issue, we think that duplicated measurements should be handled in SegmentStatistics. What do you think? Do you see a way to filter duplicates in SegmentStatistics during or after statistics computation?

@chribaue
Copy link
Collaborator

When we developed the SegmentStatistics module with the plugin mechanism and support for handling DICOM meta-data we had a lengthy discussion related to duplicate measurements: #161

It would be easy to add to SegmentStatistic exportToTable a separate Flag dicomExportReady that you could use from the QR module, but if the user selects certain plugins and measurements and they don't show up in the results table, the user would get confused and think something is not working. So we need to provide the user feedback somewhere about the issue.

I still think of it as a DICOM SR export issue that should get handled during SR export where the user gets feedback "Successfully exported SR" with potential warnings:
"Warning: Measurement ... not exported because necessary DICOM information not provided by plugin"
"Warning: Duplicate measurement types with different results! Only wrote first"

@fedorov
Copy link
Member Author

fedorov commented Jan 11, 2018

@chribaue I see your argument.

I don't think we should add dicomExportReady flag. But I was just thinking that we can have separate measurements group for individual plugins, and use Measurement Method concept (line 1 in http://dicom.nema.org/medical/dicom/current/output/chtml/part16/chapter_A.html#sect_TID_1419) to code the plugin that was used to perform the calculation. But I also think I would first want to consult with @dclunie. Let me write an email to him, or maybe better discuss at the next QIICR call. Let's postpone this issue until then.

@fedorov
Copy link
Member Author

fedorov commented Jan 11, 2018

Maybe it is even better to describe the plugin used to generate the measurement in the measurment modifier (line 6 of http://dicom.nema.org/medical/dicom/current/output/chtml/part16/chapter_A.html#sect_TID_1419). We can have unlimited number of such modifiers per measurement. You can also see an example of how this is communicated in the parameter file to the converter here: https://github.com/QIICR/dcmqi/blob/master/doc/examples/sr-tid1500-ct-liver-example.json#L63-L87

@che85 che85 removed this from the NAMIC-Winter2018 milestone Jan 11, 2018
@chribaue
Copy link
Collaborator

@fedorov both approaches sound reasonable to me. In the spirit of data provenance, we want to maintain which software/measurement method was used to produce the results. @dclunie is definitely the person to consult on how to do that.

I just though of two points we should keep in mind:

  • How will these changes impact dcmqi's interface?
  • How will this extra information be interpreted by other DICOM compatible non-Slicer software?

@fedorov
Copy link
Member Author

fedorov commented Jan 12, 2018

  • How will these changes impact dcmqi's interface?

Those features are already supported by dcmqi, so should be fine.

  • How will this extra information be interpreted by other DICOM compatible non-Slicer software?

We don't know! The only DICOM compatible software that supports SR TID 1500 is ePAD, and they use dcmqi! We are literally on the cutting edge here establishing the pattern of encoding this kind of things. It would be great if there were more tools or at least bigger community to discuss those ideas.

@fedorov
Copy link
Member Author

fedorov commented Jan 23, 2018

related to #217

@fedorov
Copy link
Member Author

fedorov commented Feb 1, 2018

After discussion, it was decided it makes sense to use TID 1409 Algorithm Identification for the purpose of specifying the plugin used.

Depends on QIICR/dcmqi#328

@fedorov
Copy link
Member Author

fedorov commented Feb 5, 2018

for the reference, QIICR/dcmqi#328 is resolved in QIICR/dcmqi#329. Example of how algorithm information can be populated in the JSON passed to dcmqi is here: https://github.com/QIICR/dcmqi/blob/master/doc/examples/sr-tid1500-example.json#L69-L72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants