ENH: Initial support of the arc beam sequence #215

MichaelColonel · 2022-08-04T11:40:24Z

A simple arc beam generator is added to Beam section of External Beam Planning module

By clicking on the "Arc beam sequence" toggle button, the arc beam sequence will be added instead of simple static beam.

cpinter · 2022-08-08T14:18:10Z

Another really great contribution!

Can you please squash the commits, at least those that have the same message?

Also it would be useful if you gave more information about how the created sequence is to be used, and in general some documentation about this feature. As detailed as it seems reasonable. Since we don't have anything better yet, please edit this page: https://www.slicer.org/wiki/Documentation/Nightly/Modules/ExternalBeamPlanning

Thank you!

cpinter

Same here as well; all I can do now is a quick read-through. I have found some things to change. Thank you!

@Sunderlandkyl @lassoan

cpinter · 2022-08-08T14:19:12Z

Beams/Logic/vtkSlicerBeamsModuleLogic.cxx

+
+  vtkMRMLScene* scene = planNode->GetScene();
+
+  vtkMRMLSubjectHierarchyNode* shNode = vtkMRMLSubjectHierarchyNode::GetSubjectHierarchyNode(scene);


scene->GetSubjectHierarchyNode() instead please

cpinter · 2022-08-08T14:19:56Z

Beams/Logic/vtkSlicerBeamsModuleLogic.cxx

+    // SAD for RTPlan, source to beam limiting devices (Jaws, MLC)
+    if (beamNode)
+    {
+//      beamNode->SetSAD(rtReader->GetBeamSourceAxisDistance(dicomBeamNumber));


Why are these commented out? If you want to keep them there please add a TODO

cpinter · 2022-08-08T14:20:57Z

Beams/Logic/vtkSlicerBeamsModuleLogic.h

@@ -60,6 +62,20 @@ class VTK_SLICER_BEAMS_LOGIC_EXPORT vtkSlicerBeamsModuleLogic :
  void UpdateTransformForBeam( vtkMRMLScene* beamSequenceScene, vtkMRMLRTBeamNode* beamNode, 
    vtkMRMLLinearTransformNode* beamTransformNode, double isocenter[3]);

+  /// Create arc delivery beam sequence 
+  /// @param initialAngle - initial angle in degrees


Please use the docstring convention that is used everywhere in Slicer and do not introduce another one. Not just here but everywhere and in the other PR as well please.

cpinter · 2022-08-08T14:21:24Z

DicomRtImportExport/Logic/vtkSlicerDicomRtReader.cxx

@@ -317,7 +319,7 @@ class vtkSlicerDicomRtReader::vtkInternal
    std::string InstitutionalDepartmentName;
    std::string ManufacturerModelName;
  };
-
+  


Unnecessary spaces

cpinter · 2022-08-08T14:23:53Z

ExternalBeamPlanning/Widgets/qSlicerDoseEngineLogic.cxx

+      proxyBeamNode->SetAndObserveTransformNodeID(proxyTransformNode->GetID());
+      return proxyBeamNode;
+    }
+  }


Should report an error if creation failed?

lassoan · 2022-08-08T20:27:14Z

Since we don't have anything better yet, please edit this page: https://www.slicer.org/wiki/Documentation/Nightly/Modules/ExternalBeamPlanning

The old Slicer MediaWiki site will be retired (made read-only) at some point. We recommend extensions to store documentation in their repository. We don't need to immediately move the existing documentation, but it would make sense to add new documentation to this repository (a common pattern is to add a Docs subfolder and within this folder add a .md file for each module).

MichaelColonel · 2022-08-10T13:16:13Z

Initial arc-beam commits were squashed.

Module file description was added

cpinter

A few more things

cpinter · 2022-09-05T11:08:27Z

Beams/Logic/vtkSlicerBeamsModuleLogic.cxx

+    // SAD for RTPlan, source to beam limiting devices (Jaws, MLC)
+    if (beamNode)
+    {
+// TODO: Add beam parameters for each angle step


I think it would be great to add an issue on GitHub for this and explain it in more detail.

cpinter · 2022-09-05T11:09:24Z

Beams/Logic/vtkSlicerBeamsModuleLogic.cxx

+    // Add beam to beam sequence node
+    if (beamNode)
+    {
+      beamSequenceNode->SetDataNodeAtValue( beamNode, std::to_string(controlPointIndex));


Is it some new convention that there is a space after the opening parentheses but none before the closing one? If there is on good reason for this please use the style that is used everywhere else in SlicerRT.

cpinter · 2022-09-05T11:10:57Z

DicomRtImportExport/ConversionRules/vtkPlanarContourToClosedSurfaceConversionRule.cxx

@@ -1383,7 +1390,7 @@ void vtkPlanarContourToClosedSurfaceConversionRule::CreateSmoothEndCapContour(vt
                        static_cast<int>(std::ceil((bounds[3] - bounds[2]) / spacing[1])),
                        1 };
  double origin[3] = { bounds[0], bounds[2], bounds[4] };
-  int extent[6] = { 0, dimensions[0] - 1, 0, dimensions[1] - 1, 0, 0 };
+  int extent[6] = { 0, dimensions[0] - 1, 0, dimensions[1] - 1, 0, dimensions[2] - 1 };


What does this change fix?

cpinter · 2022-09-05T11:11:58Z

DicomRtImportExport/Logic/vtkSlicerDicomRtReader.h

@@ -94,6 +95,15 @@ class VTK_SLICER_DICOMRTIMPORTEXPORT_LOGIC_EXPORT vtkSlicerDicomRtReader : publi
  /// Get radiation type (primary particle) for a given beam
  const char* GetBeamTreatmentDeliveryType(unsigned int beamNumber);

+  /// Check if beam control point sequence is an arc beam delivery
+  /// @param beamNumber - beam number


Fix this please as you did the rest. Thanks

cpinter · 2022-09-05T11:15:02Z

Thanks for creating the markdown page! @lassoan how do you think we should make it available? Just link it from the wiki for now?

lassoan · 2022-09-05T11:52:53Z

Thanks for creating the markdown page! @lassoan how do you think we should make it available? Just link it from the wiki for now?

The easiest is to link it from the wiki. It could be also nice (and not too much work) to create a main documentation page in github (just a ti-level RADME.md with about the same content as the wiki main page). Then, page by page, documentation could be moved from the wiki. Whenever a page is moved then the content of the page on the wiki would be replaced by a message that it is moved to Github and the links from the main pages on both wiki ans Github would be updated to the new location.

After a while readthedocs could be set up to generate a nice, versioned rendering of the documentation. (Github pages could be kept for rendering the version-independent part of the documentation, as it is already done now.)

cpinter · 2024-06-28T10:42:38Z

Hi @MichaelColonel I just saw this stale PR. Is this still on your radar? Can we help in anything to get this through?

MichaelColonel · 2024-06-28T11:43:38Z

I will try to update this PR, so it can be merged.

MichaelColonel · 2024-07-03T08:31:26Z

I've updated the PR, but the update is still under process...

cpinter · 2024-07-09T09:18:07Z

Thanks a lot @MichaelColonel! I'll wait until you say it's ready. I think I'll merge it after a very quick review, and then check the code in detail when integrating the change we are doing (working on some IEC related refactoring around the Beams and REV modules).

Do you have some (phantom or anonymized) data that I could try this with? I'm not aware of any VMAT data in the SlicerRtData repo, it would be great to upload it there as well.

MichaelColonel · 2024-07-10T10:23:20Z

This PR is ready for a review and merging. If there is a conflict between commits, let me know.

cpinter · 2024-07-11T08:10:57Z

Do you have some (phantom or anonymized) data that I could try this with? I'm not aware of any VMAT data in the SlicerRtData repo, it would be great to upload it there as well.

MichaelColonel · 2024-07-11T08:14:55Z

Do you have some (phantom or anonymized) data that I could try this with? I'm not aware of any VMAT data in the SlicerRtData repo, it would be great to upload it there as well.

No, I don't. I couldn't find any.

cpinter · 2024-07-11T08:16:31Z

No, I don't. I couldn't find any.

Then how do you know the code works? If it has not been tested at all I don't think we can integrate it.

Also, can you please do a rebase so that we can do a meaningful review against today's SlicerRT? Thank you!

cpinter · 2024-07-16T11:53:49Z

@MichaelColonel I'll leave on vacation in two days and we could get this sorted out before that if there was some data to test this with.

Were you able to test this on any VMAT data? How do you know that it works?

MichaelColonel · 2024-07-16T12:30:22Z

I think it's easier to close this RP for a while. I can try to generate RTPlan file with DCMTK and test the result when all the changes in IEC logic will be settled.

cpinter · 2024-07-16T12:39:16Z

Thanks for the quick answer!

I just integrated the IEC changes, I believe it's all done.

Please be aware that the DRR module may need fixing: #250

About this PR, I'd love to see the arc beam support in soon. On my side I'll try to ask for a VMAT plan from the local hospital we work with (although it won't be fast unfortunately).

Also FYI I'm planning to do some actual work in SlicerRT in the next months, so if you have plans maybe it would be good to coordinate. Do you have any plans to work on SlicerRT in the near future? If so do you want to meet and discuss? (I'll be on vacation from Thursday but all available in August)

Thank you!

MichaelColonel · 2024-07-22T10:47:27Z

Do you have any plans to work on SlicerRT in the near future?

Nothing in particular, maybe some bug fixes and minor improvements. At least i'm not going to create a new module.

Michael Colonel added 4 commits March 21, 2022 15:37

ENH: Initial beam arc sequence

0f7fcf6

ENH: Initial beam arc sequence

f926e2a

ENH: Initial beam arc sequence

36f5b73

ENH: Initial beam arc sequence

c560fab

cpinter requested changes Aug 8, 2022

View reviewed changes

MichaelColonel force-pushed the arc-beam branch from b99b16e to 0a3415c Compare August 10, 2022 13:13

cpinter requested changes Sep 5, 2022

View reviewed changes

Mikhail Polkovnikov added 3 commits July 2, 2024 17:07

ENH: Initial beam arc sequence

e9b6e98

ENH: Initial beam arc sequence

1ea1d22

ENH: Initial beam arc sequence

5248e63

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Initial support of the arc beam sequence #215

ENH: Initial support of the arc beam sequence #215

MichaelColonel commented Aug 4, 2022

cpinter commented Aug 8, 2022

cpinter left a comment

cpinter Aug 8, 2022

cpinter Aug 8, 2022

cpinter Aug 8, 2022

cpinter Aug 8, 2022

cpinter Aug 8, 2022

lassoan commented Aug 8, 2022

MichaelColonel commented Aug 10, 2022

cpinter left a comment

cpinter Sep 5, 2022

cpinter Sep 5, 2022

cpinter Sep 5, 2022

cpinter Sep 5, 2022

cpinter commented Sep 5, 2022

lassoan commented Sep 5, 2022

cpinter commented Jun 28, 2024

MichaelColonel commented Jun 28, 2024

MichaelColonel commented Jul 3, 2024

cpinter commented Jul 9, 2024

MichaelColonel commented Jul 10, 2024

cpinter commented Jul 11, 2024

MichaelColonel commented Jul 11, 2024 •

edited

Loading

cpinter commented Jul 11, 2024

cpinter commented Jul 16, 2024

MichaelColonel commented Jul 16, 2024

cpinter commented Jul 16, 2024

MichaelColonel commented Jul 22, 2024


		vtkMRMLScene* scene = planNode->GetScene();

		vtkMRMLSubjectHierarchyNode* shNode = vtkMRMLSubjectHierarchyNode::GetSubjectHierarchyNode(scene);

ENH: Initial support of the arc beam sequence #215

Are you sure you want to change the base?

ENH: Initial support of the arc beam sequence #215

Conversation

MichaelColonel commented Aug 4, 2022

cpinter commented Aug 8, 2022

cpinter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lassoan commented Aug 8, 2022

MichaelColonel commented Aug 10, 2022

cpinter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpinter commented Sep 5, 2022

lassoan commented Sep 5, 2022

cpinter commented Jun 28, 2024

MichaelColonel commented Jun 28, 2024

MichaelColonel commented Jul 3, 2024

cpinter commented Jul 9, 2024

MichaelColonel commented Jul 10, 2024

cpinter commented Jul 11, 2024

MichaelColonel commented Jul 11, 2024 • edited Loading

cpinter commented Jul 11, 2024

cpinter commented Jul 16, 2024

MichaelColonel commented Jul 16, 2024

cpinter commented Jul 16, 2024

MichaelColonel commented Jul 22, 2024

MichaelColonel commented Jul 11, 2024 •

edited

Loading