SlicerRt / SlicerRT

Open-source toolkit for radiation therapy research, an extension of 3D Slicer. Features include DICOM-RT import/export, dose volume histogram, dose accumulation, external beam planning (TPS), structure comparison and morphology, isodose line/surface generation, etc.
https://slicerrt.org
127 stars 60 forks source link

Add support for multi-leaf collimators #115

Open cpinter opened 5 years ago

cpinter commented 5 years ago
cpinter commented 4 years ago

@MichaelColonel This is where the MLC related work should be tracked.

So what exists in SlicerRT right now is a way to set the leaf positions https://github.com/SlicerRt/SlicerRT/blob/master/Beams/MRML/vtkMRMLRTBeamNode.h#L101-L105 However it uses double array nodes, and I think it would be much better using table nodes (vtkMRMLTableNode). Do you know how are MLC positions stored in DICOM?

There is also some code that generates the beam model from the MLC positions, see https://github.com/SlicerRt/SlicerRT/blob/master/Beams/MRML/vtkMRMLRTBeamNode.cxx#L483-L574 I think this should be OK, just need to change all to use table nodes, and add importing MLCs from DICOM (or whatever feature you need around MLCs) (https://github.com/SlicerRt/SlicerRT/blob/master/DicomRtImportExport/Logic/vtkSlicerDicomRtReader.cxx#L637-L640)

gregsharp commented 4 years ago

MLC positions are stored as a sequence of control points. MLC positions may change between control points. Each control point also specifies the gantry angle, collimator angle, jaw positions, and cumulative MUs (and several other things as well).

The plan also contains information about the number of MLC leaves and the widths of those leaves. However, it does not contain all needed information, for example leaf position limits.

cpinter commented 4 years ago

Thanks, @gregsharp ! What is the format of the MLC positions? Just a number for each leaf for each control point?

gregsharp commented 4 years ago

Correct. A single number for each leaf position at each control point.

Regarding using a vtkMRMLTableNode, that would only be for the "Current leaf position"? Or do you envision a sequence of vtkMRMLTableNode for multiple control points?

cpinter commented 4 years ago

You're probably right and we should put all this in a sequence.

lassoan commented 4 years ago

Storing in a sequence node would have the advantage that you would get browsing between time points for free. You could add any other nodes (markup fiducials, transforms, etc) to change in sync.

Potential inconvenience is that each table would contain information for only one time point, so if you want to view/edit multiple time points then you would need to switch between them, add multiple browser nodes, or temporarily export to a merged table.

MichaelColonel commented 4 years ago

I worked with MLC in DICOM a couple of times. Here from dcmdump:

(300a,00b6) SQ (Sequence with undefined length #=3)     # u/l, 1 BeamLimitingDeviceSequence
  (fffe,e000) na (Item with undefined length #=4)         # u/l, 1 Item
    (300a,00b8) CS [MLCX]                                   #   4, 1 RTBeamLimitingDeviceType
    (300a,00ba) DS [538.0]                                  #   6, 1 SourceToBeamLimitingDeviceDistance
    (300a,00bc) IS [60]                                     #   2, 1 NumberOfLeafJawPairs
    (300a,00be) DS [-200.0\-190.0\-180.0\-170.0\-160.0\-150.0\-140.0\-130.0\-120.0\-11... # 354,61 LeafPositionBoundaries
  (fffe,e00d) na (ItemDelimitationItem)                   #   0, 0 ItemDelimitationItem
(fffe,e0dd) na (SequenceDelimitationItem)               #   0, 0 SequenceDelimitationItem

MLC has 60 leaves, 61 leaf boundary values as pairs (-200., -190.) (-190., -180.) ....

Position in one control point:

    (300a,011a) SQ (Sequence with undefined length #=3)     # u/l, 1 BeamLimitingDevicePositionSequence
      (fffe,e000) na (Item with undefined length #=2)         # u/l, 1 Item
        (300a,00b8) CS [MLCX]                                   #   4, 1 RTBeamLimitingDeviceType
        (300a,011c) DS [-105.0\-105.0\-105.0\-105.0\-105.0\-105.0\-105.0\-105.0\-105.0\-10... # 786,120 LeafJawPositions
      (fffe,e00d) na (ItemDelimitationItem)                   #   0, 0 ItemDelimitationItem
    (fffe,e0dd) na (SequenceDelimitationItem)               #   0, 0 SequenceDelimitationItem

There are 120 positions, one for each leaf, first 60 for one side, last 60 for the other side.

MichaelColonel commented 4 years ago

Importing MLCs from DICOM in vtkSlicerDicomRtReader. Some general questions.

  1. BeamEntry class members are only for control point sequence data (MLC positions) or not? https://github.com/SlicerRt/SlicerRT/blob/master/DicomRtImportExport/Logic/vtkSlicerDicomRtReader.cxx#L93-L131
  2. Where should i store MLC data from BeamLimitingDeviceSequence? Should it be static members in BeamEntry class, or separate class defining MLC parameters?
  3. What public methods must be added into the vtkSlicerDicomRtReader header to get access to the MLC parameters and MLC positions within control points?
  4. Can STL containers be used in the header?

Best regards, Mikhail

cpinter commented 4 years ago
  1. Since we haven't supported multiple control point yet, it's the parameters of the beam for that one control point. Maybe I don't fully understand the question.
  2. Neither I think. Static members should only be used for application-wide single data objects such as attribute names and such. Also, MLCs positions is not complex data enough to warrant a separate class (just a vector of pairs). During loading, I think it should go to the BeamEntry, and then the import logic should set it in a member of the vtkMRMLRTBeamNode class (exactly where it is currently, but instead of a double array node it should be table node, as I suggested above)
  3. I'd say GetBeamMultiLeafCollimatorPositions(unsigned int beamNumber, std::vector)
  4. Yes, but instead of returning it, I think it's best to have it as an argument that is cleared and filled in the function.
MichaelColonel commented 4 years ago

The reason for the first question is that one multi leaf collimator definition from BeamLimitingDeviceSequence, "MLCX" for example, can have a number of positions or openings within ControlPointSequence.

These members will be added to the BeamEntry double SourceToBeamLimitingDeviceDistance; unsigned int NumberOfLeafJawPairs; std::vector< double > LeafPositionBoundary; // from BeamLimitingDeviceSequence std::vector< double > LeafJawPositions; // from BeamLimitingDevicePositionSequence

What is a better way to store boundaries and positions?

  1. Raw std::vector of values from DICOM;
  2. Formatted std::vector< std::pair< double, double > > of leaf pairs.
cpinter commented 4 years ago

It is hard to tell before starting implementation. I think you can start with either, and if you see that there are more disadvantages than advantages, then we can always switch. This is a small enough detail that it does not need to start as final. I think what is very important is to decide what data goes to which class, but it seems we have an agreement in that. If others have a preference, please chime in. Thanks!

gregsharp commented 4 years ago

My 2 cents:

The below are not part of a control point (they do not change).

double SourceToBeamLimitingDeviceDistance;
unsigned int NumberOfLeafJawPairs;
std::vector< double > LeafPositionBoundary; // from BeamLimitingDeviceSequence

But the below is part of the control point. You probably want a separate class to describe what is contained in a control point, because eventually you will have a vector of control points.

std::vector< double > LeafJawPositions; // from BeamLimitingDevicePositionSequence
cpinter commented 4 years ago

@gregsharp True, however, if we include adding the control points feature to this topic, then it will reach too far. If @MichaelColonel does not need more than one control point, then I suggest keeping it simple for now, and building on it later when generalizing for multiple control points.

MichaelColonel commented 4 years ago

I don't want to break compatibility to the code that works, since i'm quite new in SlicerRT. But in the end, each beam must have multiple control points.

Something like

class ControlPointEntry { public: // Collimators positions, openings: "X", "Y", "ASYMX", "ASYMY", "MLCX", "MLCY" unsigned int ControlPointIndex; double LeafJawPositions[2][2]; // "X", "Y" double LeafJawPositionsASYM[2][2]; // "ASYMX", "ASYMY" std::vector< double > LeafJawPositionsMLCX; std::vector< double > LeafJawPositionsMLCY; };

class BeamEntry { public: // Collimators definitions: "X", "Y", "ASYMX", "ASYMY", "MLCX", "MLCY" int NumberOfControlPoints; std::vector< ControlPointEntry > ControlPointSequenceVector; };

std::vector< BeamEntry > BeamSequenceVector;

MichaelColonel commented 4 years ago

I've mostly done with loading MLC data in DicomRtReader for both RTPlan and RTIonPlan. Only one control point per beam for start.

Where the vtkMRMLTableNode for MLC opening data should be created?

cpinter commented 4 years ago

Thanks for keeping us in the loop, I really appreciate it!

Creating the table node should be where you read in the MLC data, in ImportExportLogic. It should be set to the beam right there too. How about you make a commit with the single control point MLCs working, I take a look at it, and suggest a way to implement multiple control points without you having to do the refactoring, but in a way that makes it easier for us to do that later?

MichaelColonel commented 4 years ago

I don't have the permission to push commit.

cpinter commented 4 years ago

Let's follow the usual GitHub workflow:

  1. Fork the SlicerRT repository
  2. Create a branch (something like "multi-leaf-collimators"
  3. Do the commits there

When ready for integration, issue a pull request.

MichaelColonel commented 4 years ago

@cpinter I opened a pull request.

cpinter commented 4 years ago

Please issue a pull request to SlicerRT/SlicerRT not cpinter/SlicerRT (which is also a fork).

If the PR is not about ion plans, please rename the branch so that it is expressive to what it contains. Thanks!

cpinter commented 4 years ago

I see now why you issued the PR in my fork, it's because you want to integrate it to ion plans.

For future reference, it's important that we don't do many things under an umbrella, because it will be very hard to track why things happened and when. If you can do the MLC related commits separately which by the way should have been committed to this ticket and not the ion one, for the same reason), then we could integrate the MLCs and the ion plans separately. It would be much cleaner. I hope it's doable. Thanks!

MichaelColonel commented 4 years ago

When the RTBeamNode creates beam polydata it doesn't use parameters from a beam limiting device sequence. It had been done just for demonstration?

cpinter commented 4 years ago

It does, just above the code you highlighted: https://github.com/SlicerRt/SlicerRT/blob/master/Beams/MRML/vtkMRMLRTBeamNode.cxx#L484-L574

MichaelColonel commented 4 years ago

WIP.

MLCX data are only data available tor testing.

MLCX -- example1 testMLC

vtkMRMLTableNode -- example1 mlcTableNode

MLCX -- example2 testMLC2

vtkMRMLTableNode -- example2 mlcTableNode2

cpinter commented 4 years ago

Looks amazing, thanks a lot!

MichaelColonel commented 4 years ago

The process of handling MLCs in beam planning, and UI for that purpose.

Should it be just editing of selected MLC positions from DICOM RT file, or the user could be able to create additional MLC and edit their positions as well?

If only editing of selected positions, which is better for user interface: plugin widget or dialog window?

cpinter commented 4 years ago

I think it's enough in the plugins if we can reference the MLC table node (by node ID), and the user should edit the MLC in the Tables module.

MichaelColonel commented 4 years ago

In that case could it be guaranteed, that the modified positions data would be valid (no intersection between leaves in pair, no meaningless values, etc).

MichaelColonel commented 4 years ago

@cpinter i'm done with initial implementation and pull request is ready for review. The beam vtkPolyData is displayed correctly when any value in MLC position table node is changed, or clicked "Edit properties..." action on a beam node. That's the only bug i have noticed.

I hope somebody could test not only MLCX but also MLCY.

Sunderlandkyl commented 4 years ago

Thanks for working on this! Csaba is in the process of moving, so I can review this pull request for you tomorrow.

Sunderlandkyl commented 4 years ago

@MichaelColonel I've merged the pull request in this commit 357d9dbbbb092b928e25b064cd8dc717a4f7948c.

What organization are you a member of? We would like to include you in our list of collaborators :slightly_smiling_face:.

MichaelColonel commented 4 years ago

Institute for High Energy Physics (IHEP)

Short Name: NRC «Kurchatov Institute» – IHEP Full Name: Institute for High Energy Physics named by A.A.Logunov of National Research Center «Kurchatov Institute»

MichaelColonel commented 4 years ago

MLC representation like XiO.

Is it possible to create 3D or 2D not just a MLC modulated beam but a MLC itself? The MLC data could be used from the beam node.

I hope there is an easy way without making a custom node for that purpose.

MLC

MichaelColonel commented 4 years ago

It was relatively easy as @lassoan have said, but only as a static model. MLC_Position_Model

lassoan commented 4 years ago

Looks nice! You can automatically update the model node whenever any parameter is changed.

MichaelColonel commented 4 years ago

I can remove DoubleArrayNode node from MLC support and store all the data within single TableNode, because of #142.

New MLC data storage in table node will be: first column - leaf pairs boundary data, second column - leaf pairs position on side 1, third column - leaf pairs position on side 2.

cpinter commented 4 years ago

@MichaelColonel This would be excellent, thank you for offering to help with this! The columns seem fine with me. Please make sure you document this clearly in comments in the code.

MichaelColonel commented 4 years ago
* Allow handling MLCs in beam planning

I've made a small module to calculate MLC position for a given RTBeamNode, target volume, and table node with defined leaf pair boundaries. Some of my colleagues want to try it without compilation and other stuff.

If such feature is needed in SlicerRT how could i integrate it with minimum problems (separate module, integrate in "Beams" module, GUI for MLC leaf pair boundaries calculation and so on)?

cpinter commented 4 years ago

When you say module what do you mean exactly? I suppose not Slicer module, otherwise you wouldn't ask this question.

I think the best would be to add it as a new logic class in the Beams module's logic folder.