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
124 stars 60 forks source link

ENH: Initial support of the arc beam sequence #215

Open MichaelColonel opened 1 year ago

MichaelColonel commented 1 year ago

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. image

cpinter commented 1 year ago

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!

lassoan commented 1 year ago

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 commented 1 year ago

Initial arc-beam commits were squashed.

Module file description was added

cpinter commented 1 year ago

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 commented 1 year ago

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 commented 2 weeks ago

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

MichaelColonel commented 2 weeks ago

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

MichaelColonel commented 1 week ago

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

cpinter commented 1 week ago

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 commented 6 days ago

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

cpinter commented 5 days ago

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 commented 5 days ago

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 commented 5 days ago

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 commented 8 hours ago

@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 commented 7 hours ago

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 commented 7 hours ago

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: https://github.com/SlicerRt/SlicerRT/issues/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!