Skip to content

Conversation

@wahln
Copy link
Contributor

@wahln wahln commented Sep 20, 2019

Dear @markbangert @eric11210 ,

this is my updated attempt at merging dev_VMAT with dev especially concerning the new optimization interface, but also dose calculation.

What I did here, is merge dev_VMAT into a copy of the current dev branch. I think that this is better than what I did before (#376 ), because know you have a better acces to the changed files within this pull request (and see the changes i did within the merge commit).
Also @markangert should be able to close (#308 ).

It seems like it runs smoothly now with the old settings, i.e. fluence Opt & DAO opt without preconditioning. It doesn't work with VMAT yet (dose calculation works, but fluence optimization is weird and DAO/VMAT doesn't work)
But anyways, I hope you now have a better basis to continue with the merge.

Some comments:

  • integration worked quite well with VMAT Optimization Problem deriving from DAO. Yet I am not clear on how to do something similar with the daoVec2Aperture stuff. I am not familiar enough with the implementation, at the moment the changes are in the OptimizationProblemDAO class within the respective functions.
  • for me siochi leaf sequencing with fails with the matRad.m script, but only when sequencing is turned on and DAO (& of course VMAT) is turned of. Yet I like the idea of putting the number of leves into pln, so I changed all sequencers to have the pln struct in their call.
  • With preconditioning enabled, DAO gives worse results than directly after sequencing.
  • I don't like that the fluence optimization now needs the stf only because the tiny part of VMAT code. Can this be handled differently?
  • Should we just write a matRad_VMAToptimization that itself calls fluence optimization and dao?
  • Merging was at some points weird (git associated some wierd code parts with each other esp in generateStf and dosecalc). I hope I didn't mess something up there.

Let me know if you have questions. I will let you know if I forgot something in the list.

P.S.: Does someone of you have a very light test script for the VMAT?

New: Siochi leaf sequencing with more fluid number of kept apertures.
Still respect constraints of min/max gantry angle spacing.
Changed the first gantry angle from 0 to non-zero.  This makes things
much easier.
Figured out problem with DAO: the leaf speed was defined as the
difference between positions, not absolute difference.  Then speed was
constrained to be between 0 and 6 cm/s, which forced unidirectional
motion, increasing the objective function.

Temporary fix: allow negative values.  Eventually, change definition of
function so that it considers the absolute difference.  Will also have
to change the Jacobian.
Fixed leaf speed constraints and Jacobian.  Siochi sequencing ready to
be pilled to dev.
# Conflicts:
#	optimization/matRad_constFuncWrapper.m
#	optimization/matRad_jacobFuncWrapper.m
Testing Revert
This reverts commit 3fe0406.
Weight to MU changed from 1 to 100.
pln.bioOptimization put into options structure
This reverts commit 79e5632.
Added leaf speed constraints back in
Changed leaf speed from mm/s -> cm/s, greatly reducing treatment time.

Added function helping to test planning capabilities.

Fixed bug in Dij sampling, when no voxels are outside of the core.
Shifted time variable to be the time over the optimized beam's arc,
rather than the time between optimized beam angles.  This makes more
sense when calculating e.g. dose rate.  Also, this will make more sense
when doing dynamic fluence calculation.
Minor tweaks and bugfixes.
Split the daoVec2ApertureInfo into 3 functions so that we didn't have to
keep running if statements on VMAT, dynamic vs. static fluence
calculation, etc.
Added option for dynamic fluence optimization for VMAT DAO, including
support for interpolated apertures.
Support for Jacobian scaling
Added support for 4D dose calculation, including deformed CT cubes and
deformation vectors.
This reverts commit de9682f.
@wahln
Copy link
Contributor Author

wahln commented May 6, 2021

The most recent updates fixed the jacobian computation and ensured that we always use the newest shape/bixel information (see #506 ), so now it is possible to use dose constraints again.
Something seems to be off when using preconditioning still.

@wahln
Copy link
Contributor Author

wahln commented May 8, 2021

Fixed the bug of the missing dij.scaleFactor in matRad_DoseProjection. I don't think that's the best solution, and we should look at it further berfore merging, but at least we should be back with a working default VMAT setting with the new structure.
So now, tasks remaining for me include

  • figure out the best and least intrusive way to include the preconditioning factor
  • integrate all other VMAT-Optimization-specific code into the VMAT Optimization Problem class
  • make sure standard DAO & VMAT are nicely separated in the new structure and VMAT reuses DAO code
  • check all new parameter definitions
  • integrate the preconditioner into standard DAO as part of the merge (bonus)

Comment on lines 56 to 58
pln.propOpt.VMAToptions.maxGantryAngleSpacing = 2; % Max gantry angle spacing for dose calculation
pln.propOpt.VMAToptions.maxDAOGantryAngleSpacing = 4; % Max gantry angle spacing for DAO
pln.propOpt.VMAToptions.maxFMOGantryAngleSpacing = 28; % Max gantry angle spacing for FMO
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters only seem to work for certain combinations of parameters.
Using the following comination throws an error:
pln.propOpt.VMAToptions.maxGantryAngleSpacing = 8; % Max gantry angle spacing for dose calculation pln.propOpt.VMAToptions.maxDAOGantryAngleSpacing = 16; % Max gantry angle spacing for DAO pln.propOpt.VMAToptions.maxFMOGantryAngleSpacing = 28; % Max gantry angle spacing for FMO
If this is intended behavior, the conditions should be tested for and a sensible error message should appear. I don't know if I induced this or it was already there before.

@github-actions
Copy link

github-actions bot commented Jul 7, 2024

This PR was automatically marked as stale it has been open 30 days with no activity. Please review/update/merge this PR.

@github-actions github-actions bot added the stale Automatic label for stale issues label Jul 7, 2024
@eric11210
Copy link
Contributor

@wahln is this the branch that you want to merge into dev?

@github-actions github-actions bot removed the stale Automatic label for stale issues label Feb 14, 2025
@wahln
Copy link
Contributor Author

wahln commented Feb 14, 2025

Hi @eric11210
yes, in the end this will be the branch to work on. I will try to update it to the latest release before merging and resolve any conflicts on the weekend, so we can work on it during the hackathon.

# Conflicts:
#	dicom/matRad_importDicomSteeringPhotons.m
#	examples/matRad_example3_photonsDAO.m
#	matRad.m
#	matRad/basedata/photons_Generic.mat
#	matRad/matRad_directApertureOptimization.m
#	matRad/optimization/@matRad_OptimizationProblemDAO/matRad_daoApertureInfo2Vec.m
#	matRad/optimization/@matRad_OptimizationProblemDAO/matRad_daoVec2ApertureInfo.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_OptimizationProblemVMAT.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_constraintFunctions.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_constraintJacobian.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_daoApertureInfo2Vec.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_daoVec2ApertureInfo.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_getConstraintBounds.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_getJacobianStructure.m
#	matRad/optimization/@matRad_OptimizationProblemVMAT/matRad_objectiveGradient.m
#	matRad/optimization/matRad_bixWeightAndGrad.m
#	matRad/optimization/matRad_daoVec2ApertureInfo_VMATrecalcDynamic.m
#	matRad/optimization/optimizer/matRad_OptimizerFmincon.m
#	matRad/sequencing/matRad_sequencing2ApertureInfo.m
#	matRad/sequencing/matRad_siochiLeafSequencing.m
#	matRadGUI.m
#	matRad_DijSampling.m
#	matRad_calcCubes.m
#	matRad_calcDoseDirect.m
#	matRad_calcDoseInit.m
#	matRad_calcPhotonDose.m
#	matRad_fluenceOptimization.m
#	matRad_generateStf.m
#	optimization/@matRad_OptimizationProblem/matRad_constraintFunctions.m
@wahln
Copy link
Contributor Author

wahln commented Feb 14, 2025

I updated to the recent dev as best I could. Most important things to do:

  • The VMAT stf generation methods need to be integrated into the new StfGenerator structure. I did the ground work for that by adding an StGeneratorPhotonVMAT which runs through. Some remaining cleanup might be necessary.
  • Adjusting and sorting the remaining added files to the new file structure
  • Checking if the dose engine structure is still compatible with the VMAT stuff
  • Checking if the optimization is still compatible
  • Sort out the options prop*. We should make sure we follow the matRad philosophy here.
  • Some quantitative changes to check if the results are correct
  • writing some tests

@eric11210
Copy link
Contributor

eric11210 commented Feb 14, 2025

@wahln Sorry I checked off one of the boxes but I cannot uncheck it...

Is there a document or guidance on the file structure/philosophy that I can review? It's been some time since I've looked at newer matRad branches.

@github-actions
Copy link

Code Coverage

Package Line Rate Health
coverage Package 1 43%
Summary 43% (8980 / 20905)

@codecov
Copy link

codecov bot commented Feb 14, 2025

❌ 7 Tests Failed:

Tests completed Failed Passed Skipped
281 7 274 5
View the top 3 failed test(s) by shortest run time
test_siochiLeafSequencing test_run_sequencing_basic
Stack Traces | 0.004s run time
matRad_siochiLeafSequencing:65 (.../matRad/sequencing/matRad_siochiLeafSequencing.m)
test_run_sequencing_basic:29 (.../test/sequencing/test_siochiLeafSequencing.m)
test_xiaLeafSequencing test_run_sequencing_basic
Stack Traces | 0.01s run time
matRad_xiaLeafSequencing:98 (.../matRad/sequencing/matRad_xiaLeafSequencing.m)
test_run_sequencing_basic:29 (.../test/sequencing/test_xiaLeafSequencing.m)
test_engelLeafSequencing test_run_sequencing_basic
Stack Traces | 0.014s run time
matRad_engelLeafSequencing:95 (.../matRad/sequencing/matRad_engelLeafSequencing.m)
test_run_sequencing_basic:29 (.../test/sequencing/test_engelLeafSequencing.m)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@wahln
Copy link
Contributor Author

wahln commented Feb 14, 2025

@wahln Sorry I checked off one of the boxes but I cannot uncheck it...

Is there a document or guidance on the file structure/philosophy that I can review? It's been some time since I've looked at newer matRad branches.

Unfortunately we haven't really written this down yet in the documentation. But here's some new principles

  • The new file structure aims at minimizing files in the root folder (everything is now in a subfolder "matRad"). In the matRad folder on the top level only the "top-level API" is exposed (e.g. generateStf, calcDose*, etc.). Subfolders keep the more specialized components. Most of the vmat geometry may thus go either into "steering" (where the stf information goes) and/or in its own folder matRad/VMAT.
  • we do not want too many prop* fields. prop* fields are now used to configure the different workflow steps, and different implementations for a certain workflow step / data structure are separated in different classes. For example, VMAT-specific stf properties should just be put in propStf and implemented in the matRad_StfGeneratorVMAT - matRad will then configure accordingly.
    Currently we have: propStf (to configure StfGenerators), propDoseCalc (to configure DoseEngines), propOpt (to configure optimization), propSeq (to configure Sequencers)
  • we do want to minimize cross-dependencies (e.g. at optimization time, we do not want to look up a propStf). This means that properties for stf generation that are needed in the optimization need to be propagated in stf / dij to have access to them later on.

@github-actions
Copy link

github-actions bot commented Feb 14, 2025

Test Results

  3 files  ±0    3 suites  ±0   34m 7s ⏱️ -5s
281 tests ±0  274 ✅  -  7  0 💤 ±0   7 ❌ + 7 
939 runs  ±0  913 ✅  - 21  5 💤 ±0  21 ❌ +21 

For more details on these failures, see this check.

Results for commit 2b4490d. ± Comparison against base commit fdf2906.

♻️ This comment has been updated with latest results.

@eric11210
Copy link
Contributor

eric11210 commented Feb 19, 2025

I'll update this comment with idea/steps as we go along. I've included the ones @wahln had above.

General aspects

  • Should we just create matRad_VMAToptimization that calls matRad_fluenceOptimization and matRad_directApertureOptimization?
    No.
  • Adjust and sort the remaining added files to the new file structure.
  • Sort out the prop* options. We should make sure we follow the matRad philosophy here.
  • Should try to minimize duplication of runVMAT type fields. E.g., it is needed in many of the prop* fields.
  • Generate reference plan from Eric's branch with known implementation to test new merge (and for ongoing testing).

Continuous aperture vs. fixed aperture
Currently, there are "if else" statements throughout any function for which there is a difference in implementation/code between continuous and fixed aperture. What is the best way to handle this? Should there be different classes for the continuous and fixed aperture cases?

Keep the "if else" statements.

matRad_VMATGantryAngles

  • Perhaps move this entire function into matRad_StfGeneratorPhotonVMAT?
    The function itself was slightly changed, and is called from within matRad_StfGeneratorPhotonVMAT. This could probably be better integrated in the future.
  • Ensure it can run with no errors with arbitrary gantry angle spacing.
  • Include warnings if strange gantry angle spacings are input.

matRad_StfGeneratorPhotonVMAT

  • Determine if anything needs to be or can be further cleaned up.
  • There are a lot of new fields for VMAT. Is the stf structure the best place to contain them?
    Fine for now.

matRad_calcDoseInfluence

  • Verify whether the dose engine structure is still compatible with the VMAT stuff.

matRad_fluenceOptimization

  • Verify that the VMAT-specific FMO stuff is still done. I.e., only performing FMO on FMO beams.

matRad_sequencing

  • Long-term: object-oriented sequencers? A lot of code is replicated...
  • Ensure sequencing still runs after previous refactoring.

matRad_directApertureOptimization

  • Ensure new optimization framework is still correct.
  • Verify that all the gradients are correct, for 4 combinations of: continuous and discrete aperture; and preconditioner true and false.
  • Verify that the delivery constraint Jacobians are correct, for continuous and discrete aperture.
  • Verify that the dosimetric constraint Jacobians are correct, for continuous and discrete aperture.
    Something is going wrong here: when I calculate the Jacobian using the finite difference approximation, it does not match well with the one that I calculate in the code. Possibly due to the preconditioner?
  • Clean up continuous vs. fixed aperture stuff.
  • Ensure preconditioning is done correctly (there may be some error?), and in the least intrusive way.
  • Integrate all other VMAT-Optimization-specific code into the VMAT Optimization Problem class.
  • Make sure standard DAO & VMAT are nicely separated in the new structure and VMAT reuses DAO code.
  • BONUS: Integrate the preconditioner into standard DAO as part of the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants