WIP: Adding DICOM-RT Dose grid summation w/ interpolation from DVHA code #164
Conversation
|
Some DICOM tags to update (per my conversation with @bastula ):
Possible to store SOPInstanceUIDs of source dose files? |
|
|
…mation * Began using Black to format code with 79 character line length * Doc string formatting updated to be consistent with other dicompyler-core files * The other_factor parameter in add, direct_sum, and interp_sum was removed in favor of multiply() and * overload
…mments * Avoid extra list creation in DoseGrid.shape * DoseGrid.scale raises UserWarning if non-uniform GridFrameOffsetVector detected * Raise UserWarning in DoseGrid.add() if mismatched DoseSummationType, DoseType, DoseUnits, ImageOrientationPatient
|
This pull request introduces 1 alert when merging a5bd38a into 13b492c - view on LGTM.com new alerts:
|
|
The unit tests are nearly complete. Let me know if any other tests should be added beyond the following:
I'm actually not sure of an appropriate way to test |
Not sure if The following properties/methods need coverage. I'm not sure if you're calling
|
|
I was thinking it might be helpful for users that sent the wrong dataset/file? I’m ok taking it out.
… On Aug 5, 2020, at 5:29 PM, Aditya Panchal ***@***.***> wrote:
@bastula commented on this pull request.
In dicompylercore/dose.py:
> + DICOM RT Dose used to determine the structure dose grid data.
+ order : int, optional
+ The order of the spline interpolation.
+ 0: the nearest grid point, 1: trilinear, 2 to 5: spline
+ """
+
+ self.ds = dicomparser.DicomParser(rt_dose).ds
+ self.order = order
+
+ self.summation_type = None
+ self.sop_class_uid = self.ds.SOPClassUID
+ self.sop_instance_uid = self.ds.SOPInstanceUID
+ self.other_sop_class_uid = None
+ self.other_sop_instance_uid = None
+
+ if self.ds.Modality == "RTDOSE":
Is this check and error necessary? I would assume if someone is using this class, they would be summing RT Dose.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Testing for
|
|
One more question/thought - what happens if two dose grids are not covering the same spatial extent for interp_sum? E.g. if one's X position values range from -10 to + 10, and the other from 0 to +20, then (if I understand correctly), the second will be assumed to be zero from -10 to 0, and the second's dose from +10 to +20 will be ignored because it doesn't match the primary's grid. (?) It seems to me the only reliable dose would be in the overlap region of the two grids. Of course it could be left to user code to check the two being added and reduce the spatial extent to the common region if desired. But it would be nice to at least document what happens for these situations. Conversely, perhaps it could be allowed to direct sum two with different extents (but same spacing, etc.) and return only the common region? And... again, if I'm being too much of a pain, any or all of this could be left for future work. |
|
You are correct, extrapolated points are assumed to have a value of zero. We could easily allow I agree it's a good idea to document this. I'll add that before we merge. As far as cropping to the overlapping region, maybe let's gauge the need for it and leave that to another release? I think users should be responsible for having their dose grid boundaries extended to near-zero values. Maybe the more appropriate solution is to raise a warning if any boundary dose is greater than some user-editable threshold (e.g., 0.01% max default maybe)? |
|
I set the boundary dose threshold to 0.1% of maximum dose. I felt this was a better approach than involving prescription dose since that's not set consistently across vendors. |
|
Thanks all of the feedback @darcymason |
|
LGTM, some minor documentation changes, but I will do that in a separate commit. |
Initial commit to add DICOM dose summation with interpolation as a part of dicompyler-core.