-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Load RT ion plans #25
Comments
An additional dataset and contributor has arisen from the Slicer community. We will attempt to add the ion plan support together. See |
I have created a branch where I added placeholders for the RT ion plan import/load part. To start, you will need to
Thank you so much and good luck! |
thanks for laying out the steps for me, |
Excellent! Please let me know if you encounter any issue :) |
I have gotten as far as trying the build stage, and I am currently getting the error shown below (towards the end of the Cmake report listed below), am not sure how to proceed. any assistance would be much appreciated. Setting C++ standard Configuring incomplete, errors occurred! |
You will need a newer CMake. Please download the latest stable. |
I think patch.exe is not bundled with CMake but with git. Probably patch.exe is not found because you have not installed git in standard location. See how to specify |
I have succesfully configured and generated using Cmake, but i am having trouble understanding the steps that I need to take using Visual Studio in order to continue the build process. For example giving the scope of what I am trying to do, should I use release or debug mode? |
In any case, I think a new CMake will be needed, since the paths show a version of 3.7, and although Slicer specifies a minimum of 3.5, I think we need a newer one. |
Open Slicer.sln, choose Debug mode (I see in your build path that you wanted debug build, and I think you may need it for the development), and start build. |
yeah i downloaded the newest stable release of CMake |
the build has been completed with a message stating "52 succeeded, 4 failed 3 skipped" However i am having trouble locating the Slicer.exe file mentioned in the Documentation, it is not in the Slicer-build folder, does this mean something as gone wrong or is there a work around. |
There should be 0 failed. Please check out the error list and tell us at least the first one. Thanks! |
i cant find the error list, but i realised that i selected the wrong complier x32 instead of x64, so I have restarted the build with the correct complier, will let you know how it goes |
The error list is a window in Visual Studio. Please find it, you will need it later. |
Interesting. These errors do not indicate any error with actual builds. Does the file ...S4D\Slicer-build\Slicer.exe exist? If so, then consider the build successful. |
yes it does i was able to run slicer |
Excellent! One hard part is done! Keep us posted about the next steps and I'll help if I can. |
Your screenshot says 15 errors. Can you please send me at least the first few? Thanks. |
Warnings are fine. This looks good, thanks! Next is to run Slicer in Debug mode with SlicerRT.
|
I have been able to run slicer with the SlicerRT extenstion and evrything works as expected. |
Great, you're making fast progress! Yes the next step will be the code. Please refer to the second part of my descriprion above, with the bullet points. You'll need to clone the branch to your computer, check out my commit so see where these parts need to be added, and try to fill the gaps. Please ask away if you have any questions! |
followed the direction listed by cpinter to modify : vtkSlicerDicomRtImportExportModuleLogic vtkSlicerDicomRtReader Re SlicerRt#25
@MichaelColonel I rebased the branch to the master, fixed the errors and the style, and pushed the new branch here: https://github.com/cpinter/SlicerRT/tree/25-add-ion-plan-support Please fork SlicerRT, add my fork as a remote, check out the branch, then do your changes, and push the updated branch to your remote. Please let me know if you have any questions. |
followed the direction listed by cpinter to modify : vtkSlicerDicomRtImportExportModuleLogic vtkSlicerDicomRtReader Re SlicerRt#25
@cpinter I forked and added remote I'm going to use testing branch, and only after testing i will merge and push the results. I would like to start with implementation of MLC support, pretending there is only one mutileaf collimator per beam. |
Sounds Great! Before you start implementing MLCs can we please agree on the way it will be done? Some MLC support was added in SlicerRT before, but it never really worked. Could we tackle the MLC work separately in its own issue please? I'll add a comment on what is there and what I would start with if I had time working on it now. It's #115 |
Bare minimum of ion plan support in vtkSlicerDicomRtReader. For a possible future setup with a fixed beam in our institute i'm going to implement some sort of a minimum support for rt ion plan and rt plan (since both have many common elements). Major part of the code that had been already done remains (with minimum refactoring of BeamEntry class). Public methods of vtkSlicerDicomRtReader will be the the same. As a template:
Some members from BeamEntry class will go to the ControlPointEntry class. What do you think about that structure? |
Thanks again for working on this and to ask our opinion! It looks great. Just one question: I don't quite understand the ReferencedBeamEntry structure. The fractions use the same beams but with different weights, and this is why we need to reference them like this? Also could you please run the automated tests to see if your changes break any existing use cases? Just run this from command line from the inner-build folder: |
Referenced beam sequence is used only for correct order order of beams from fraction to fraction, but any critique is useful. I have run tests on my local PC using master branch after MLC support had been added: The following tests FAILED: |
Just FYI, the vtkSlicerSegmentMorphologyModuleLogicTest and vtkSlicerDoseVolumeHistogramModuleLogicTest failures are due to some changes loading Segmentations. I'm working to fix the problem. They shouldn't be related to the MLC changes. |
Thanks, @Sunderlandkyl ! @MichaelColonel I asked because if it's only for referencing a beam from within the class, then the BeamEntry pointer can be used. If it's for referencing from outside the class, then the beam number is enough. I saw the vector<pair<double, int> > and thought the double meant the weight of the beam in the fraction ("Cumulative Meterset Weight (300A,0134)"). So if additional information is needed other than identifying the beam, then this reference structure has a place, but otherwise it seems redundant. |
It looks like for correct ion plan support i need to add a MRML node for ion beam, because some elements have additional parameters. SAD becomes VSAD with x and y component, scan mode without jaws and MLC, etc. What is a better way?
When loading beams for ion plan, additional code will be added to LoadExternalBeamPlan method to check if RTPlan or RTIonPlan have been loaded.
|
Again, thanks for checking with us! Given that many beam features are the same in an ion beam as in the original beam class (geometry etc.), and that we should be able to identify all beams by doing a type check against the beam class, I think the ion beam should definitely be a subclass of the beam, so option 2. |
HIT examples from SlicerRtData all have modulated ion beam, with scan spot position map, and i am going to calculate rectangle borders of the position map just like jaws to draw the simplest beam border. Do you have any other suggestions how to make a correct poly data for beam visualization? |
Sorry I'm not sure what the question is exactly. Also can you please briefly describe what are scan spot position maps? Thank you! |
The spot positions are for a scanned (rather than passively scattered) proton beam. I think that the enclosing rectangle sounds fine. Maybe increase it by the beam sigma, otherwise a single spot would show up as just a point. Otherwise you can try to create a circle for each spot (expanded by sigma) and make their union? |
WIP @cpinter I agree, it's not the best way to describe a problem, next time i will try be more accurate! For testing i use https://discourse.slicer.org/t/beam-data-in-slicer-rt-not-loading/5718/10 data. Each control point point is loaded as a separate beam, some parameters for control points were taken from Plastimatch. @gregsharp rectangle enclosing is enough in my case, and i will think about creating union of scanning spots, but number of scan spots can be several hundreds for each control point of modulated ion beam. PS. There is a mention in code about beams group nodes. Is it a concept for the future?
|
@MichaelColonel First of all, this looks awesome. Outstanding work! Thank you so much for your contribution to the community. For clinical beams, enclosing rectangle is perfectly great. Early adopters will be physicists for whom single spot plans are common, and yes, well, you know the rest. Yes I agree, the mention in the code is future work. |
@cpinter I got to this point (finally). So I was able to build and run 3D slicer. exe and building now the slicerRT extension in visual studio. But I am not sure how point 1 needs to be executed. What do you mean with the additional module paths? Do I need to copy it inside the folder of my slicer.exe? Thanks for your response! |
You need to add the fodlers in Application Settings / Additional module paths, either by clicking Add or drag&drop. |
Thanks you very much for your reply. When loading module "BatchStructureSetConversion" , the dependency "DicomRtImportExport" failed to be loaded. |
I have some ideas what the problem could be but I'm missing the context and would like to make sure your question is resolved in the right place. Is this related to loading RT ion plans in some way? Am I forgetting something? If this is a new issue then I suggest opening a new topic on the https://discourse.slicer.org/ forum. |
This issue here is about implementing the feature for loading ion plans. Until it is closed it is not available in the SlicerRT release. Please either give it a bit more time until we close the issue, or build the topic branch instead of SlicerRT master. For using a manually built extension please ask on the forum so that here we can focus on the development of the feature. |
@mdsainth Update: Sorry, I just realilized that the comments and commits were one year older than I thought - the typical new year confusion :) In any case, about questions please use the forum. Here on GitHub we mainly manage the development. Thanks! |
Thanks! Works perfectly 👍 |
I'm making a better visualization of the scanning beam, and i have a question about how to do better. Here is a screenshot of HIT test data where scan spot position map has more than one segment (an eye and a smile). Now i use vtkAppendPolyData filter to show all single spots. Do you know any way to unite correctly spots from each segment? The vtkBooleanOperationPolyDataFilter will unite all the spots into the one big blob. |
vtkBooleanOperationPolyDataFilter often fails randomly for simple, completely valid inputs. I would not rely on it. vtkbool library can be used as an alternative. While it is not perfect, it works much better than vtkBooleanOperationPolyDataFilter and it has an active, dedicated maintainer, so there is someone to report errors to or ask advice from. We have not yet integrated vtkbool into Slicer core, but initial tests look promising, so if VTK9 upgrade in Slicer will be complete then we will make it available in the Slicer core. For now, it is built as part of Sandbox extension. See some more details here: https://discourse.slicer.org/t/new-experimental-feature-boolean-operations-union-intersection-difference-on-meshes/16048/7 |
Ion plan loading is implemented. Improvements and bugs can be discussed in new tickets as needed. |
Example datasets:
Migrated from https://app.assembla.com/spaces/slicerrt/tickets/419-load-rt-ion-plans/details
The text was updated successfully, but these errors were encountered: