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

ENH: revise initialization of TrackingUID #365

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fedorov
Copy link
Member

@fedorov fedorov commented Oct 26, 2018

If not provided by the user, initialize it. This can't hurt, but will come handy if measurements are done later using this segmentation.

If not provided by the user, initialize it. This can't hurt, but will come
handy if measurements are done later using this segmentation.
fedorov added a commit to AIM-Harvard/pyradiomics that referenced this pull request Oct 26, 2018
TrackingIdentifier and TrackingUniqueIdentifier are required. If not specified,
TrackingUniqueIdentifier will be initialized automatically by dcmqi. If either
one is available in SEG, propagate to link with the measurements.

Related PR for dcmqi will make sure TrackingUniqueIdentifier is always created
for SEG segments. It does not hurt, but will become useful if those segments are
later used for measurements. See QIICR/dcmqi#365.
@fedorov
Copy link
Member Author

fedorov commented Oct 29, 2018

This can't hurt

This potentially can hurt, since if not initialized by the user explicitly in a longitudinal tracking situation, TrackingUID will be different even if the structure is the same, and the user may be unaware of this. It can also happen that the TrackingUID in SR may need to be different from SEG, which will become messy... No longer 100% sure this is a good idea!

@pieper
Copy link
Member

pieper commented Oct 29, 2018

if not initialized by the user explicitly in a longitudinal tracking situation, TrackingUID will be different even if the structure is the same

I'd say this is appropriate - if the the user doesn't explicitly affirm that the lesions are the same then the should have different UIDs.

@fedorov
Copy link
Member Author

fedorov commented Oct 29, 2018

Well, I think the question becomes what is the significance of something not being said. Let's say we have two time points with segmentations, and there are three situations for SEG (all of which are valid):

  1. tracking UID is assigned, and is different
  2. tracking UID is assigned, and it is the same
  3. tracking UID is not assigned

Then let's say someone wants to do measurements over that SEG, and that someone wants to assign consistent UID for the timepoints. If option 2 or 3 above is followed, this is possible, but otherwise producer of SR is locked between two unfavorable choices:

  • violate the standard by assigning the same TrackingUID in SR (since it says "[Tracking UID] will have the same value as the value of the (112040, DCM, "Tracking Unique Identifier") content item in SR instances that reference this Segment in this Segmentation Instance.")
  • propagate TrackingUID from SEG, and not express the longitudinal connection between the items.

The problem is SEG and SR may be created by different entities, with SEG generator not aware of consequences of not populating an optional parameter.

I am leaning towards not initializing TrackingUID in SEG by default, unknown to the user.

If neither TrackingID nor TrackingUID are initialized, then do nothing.

If TrackingID is initialized, generate TrackingUID and print a warning
alerting user that they should pay attention in case they encode longitudinal
data.

If TrackingUID is initialized, but TrackingID is not, reuse SegmentLabel as
TrackingID, and alert the user about that.
@fedorov fedorov changed the title ENH: always initialize TrackingUID ENH: revise initialization of TrackingUID Oct 29, 2018
@fedorov
Copy link
Member Author

fedorov commented Oct 29, 2018

TODO:

  • revise tid1500writer to check TrackingUID from SEG and propagate/issue error/warning if mismatched

@jriesmeier
Copy link
Member

@fedorov Regarding the "tid1500writer": The setReferencedSegment() method from TID 1411 class also sets (i.e. copies) the tracking information from the referenced segment if parameter "copyTracking" is true (which is the default). Not sure whether this information is helpful, but I just wanted to let you know.

@fedorov
Copy link
Member Author

fedorov commented Nov 8, 2018

Thank you Jörg, this is indeed helpful to know!

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.

None yet

3 participants