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

In VTK <= 8.2, unused points weren't used in bounds calculation, while they are used in VTK >= 9.0. This caused issues with the creation of the end cap contour, since the image extent was incorrect. Fixed by removing unused points. Fixes a number of failing DVH tests.

Fix crash that is caused by the fact that ApplicationLogic is not set for generic tests. The "CopyNode" function is static, so the issue was fixed by using the method statically.

Fix crash by checking if beamsLogic exists before accessing.

The test data was no longer available since "slicer.kitware.com/midas3" was retired in August 2021. Fixed by pointing the URL to the raw files in SlicerRTData.

On Windows, CLI build directories were not added to the list of additional module directories. This resulted in missing CLI modules, causing qSlicerDrrImageComputationModuleGenericTest to fail.

The default source-axis distance and aperture distance was invalid for proton dose calculation. Fixed by changing the SAD back to the original default.

The isodose surface generated with VTK9 is slightly different around the holes in the bottom of the mesh. This causes a small volume difference between the baseline and generated surface. The surface is the same in all other areas, so there doesn't seem to be an issue. Fixed by updating the baseline.

Due to updates in VTK, TextActorAnchors needs to be initialized in LayoutTicks to not cause a crash.

Update Plastimatch to replace std::auto_ptr with std::unique_ptr. Change use of std::not1 and std::ptr_fun to use a lambda function instead. Fix SlicerRt#212

Update plastimatch to remove dynamic exception compile errors. Re SlicerRt#213

Update plastimatch to remove register storage specifier. Re SlicerRt#213

1. Logic debug information was removed 2. ExternalBeamPlanning module description file was created

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.

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

Michael Colonel and others added 19 commits August 10, 2022 16:21

ENH: Initial beam arc sequence

30203ff

BUG: Fix failing Isodose module generic tests

909a556

Fix crash that is caused by the fact that ApplicationLogic is not set for generic tests. The "CopyNode" function is static, so the issue was fixed by using the method statically.

BUG: Fix failing Beams module generic tests

3aaf917

Fix crash by checking if beamsLogic exists before accessing.

BUG: Fix DicomRtImportTest

2952e71

The test data was no longer available since "slicer.kitware.com/midas3" was retired in August 2021. Fixed by pointing the URL to the raw files in SlicerRTData.

BUG: Update PlmProtonDoseEngineTest data url

e33f1e6

BUG: Fix qSlicerDrrImageComputationModuleGenericTest on Windows

7d45aff

On Windows, CLI build directories were not added to the list of additional module directories. This resulted in missing CLI modules, causing qSlicerDrrImageComputationModuleGenericTest to fail.

BUG: Fix failing PlmProtonDoseEngineTest

53af755

The default source-axis distance and aperture distance was invalid for proton dose calculation. Fixed by changing the SAD back to the original default.

BUG: Update URL in BatchStructureSetConversion test

636519a

BUG: Fix ctCreate not running if runCTCreateAndDoseXYZnrcIndex selected

6775c74

ENH: Update window/level of relative isodose to match color table

0293cbe

BUG: Fix crash when showing isodose scalar bar

b98b655

Due to updates in VTK, TextActorAnchors needs to be initialized in LayoutTicks to not cause a crash.

COMP: Fix compile errors with Cxx17

298eee3

Update Plastimatch to replace std::auto_ptr with std::unique_ptr. Change use of std::not1 and std::ptr_fun to use a lambda function instead. Fix SlicerRt#212

COMP: Fix C++17 dynamic exception error

b994027

COMP: Fix C++17 compile errors on Mac/Linux

41d2e1c

Update plastimatch to remove dynamic exception compile errors. Re SlicerRt#213

COMP: Update plastimatch to Fix C++17 compile errors on Mac

408b2ec

Update plastimatch to remove register storage specifier. Re SlicerRt#213

ENH: Add option to OrthovoltageDoseEngine to erase dose outside segment

0294eae

ENH: Dynamic arc beam sequence GUI and logic upgrades

0a3415c

1. Logic debug information was removed 2. ExternalBeamPlanning module description file was created

cpinter requested changes Sep 5, 2022

View reviewed changes

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