-
Notifications
You must be signed in to change notification settings - Fork 177
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
Pareto navigation #669
base: dev_varRBErobOpt
Are you sure you want to change the base?
Pareto navigation #669
Conversation
Objectives and constraints are now stored as variables of the optimization problem. This takes away a lot of the redundant operations in the optimization function that loop over the cst otherwise. Also changed the objective function to now calculate all objective function values in a separate function and then sum them up afterwards. First changes for Pareto optimization were also added with the normalization options
Readded function head to getConstraintBounds and more documentation.
- Added plan navigation functionality - Fixed bug with normals in higher dimensions - Added test scripts - Option to calculate voxels on dose Grid
Fixed some changes mentioned in the pull request regarding normalization
- Renamed type to scheme for normalization - Moved normalization to optimation problem for Gradient (not entirely happy) - Some cleanup
Thanks for that PR. |
Changed converge criteria and added warnings to Pareto function
Is this similar to https://www.ncbi.nlm.nih.gov/pmc/articles/PMC8526351/ ? Thanks! |
No, this uses a different algorithm that generates a set of Pareto optimal plans (https://papers.ssrn.com/sol3/papers.cfm?abstract_id=1427721) and allows for interactive exploration of the Pareto surface after computation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a rough first review. I think that the pareto stuff can be more directly integrated.
This means you should not have a "Pareto" folder on the top level. The UI should follow the Widget classes (look in the gui folder). the optimization functions as well as the functions for generating points etc. can go into the optimization folder (you can make a pareto subfolder there).
@@ -24,17 +24,27 @@ | |||
|
|||
properties | |||
BP | |||
normalizationScheme = struct('scheme','none'); % used in pareto optimization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this a struct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the normalization I need to set upper and lower bounds that I store as further fields of the structure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then they should probably be initialized to NaN or some default value as well here.
optimization/@matRad_OptimizationProblem/matRad_OptimizationProblem.m
Outdated
Show resolved
Hide resolved
Pareto/tests/matRad_ParetoExample.m
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function names / script names should not start with a capital letter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, isn't this an exampkle and not a test?
@@ -0,0 +1,74 @@ | |||
function [k,facets] = matRad_ParetoSurfFromFacets(fVals) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment on capitalization
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems weird, that this file is changed in your PR. But the change is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure myself, why this was changed. I think this is already present on the main branch and it was updated when I synced my fork.
matRad_fluenceOptimization.m
Outdated
@@ -263,7 +263,7 @@ | |||
backProjection.scenarioProb = pln.multScen.scenProb; | |||
backProjection.nominalCtScenarios = linIxDIJ_nominalCT; | |||
|
|||
optiProb = matRad_OptimizationProblem(backProjection); | |||
optiProb = matRad_OptimizationProblem(backProjection,cst); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I would change this to have the cst first, because in the future it will be possible to have multiple projections (multiple quantities)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose this approach since I was not sure if there are cases where the matRad_OptimizationProblem is called in other places than matRad_fluenceOptimization (and my Pareto optimization). So the idea was to allow "backwards" compatibility for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should follow the Widget mechanism (check the gui folder). Create a widget derived from matRad_Widget here, you may ask @amitantony for help with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes :D
- Changed naming conventions - Some first moving of files
- Created a preProcessing function that is called at the start of fluenceOptimization and lexicographicOptimization to unify the process - Updated objectives to allow for change to lexicographic objective _ Added support for priority lists
# Conflicts: # MatRad_Config.m # matRad_calcCubes.m # matRad_fluenceOptimization.m # optimization/optimizer/matRad_OptimizerIPOPT.m
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I gave it a thorough review, see the individual comments.
options = optimoptions('linprog','Display','iter'); | ||
%options = optimoptions('linprog','Algorithm','interior-point','Display','iter'); | ||
v = linprog(f,A,b,Aeq,beq,lb,ub,options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this requires the Optimization Toolbox. Is there a way to solve it (maybe slower) with standard Matlab tools? Then we could have an if / else setup. Anyway, you should check for a license using license('test',...) and throw a descriptive error if not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Option might be to check for the optimization toolbox when starting the Pareto GUI and if it is not available only allow to navigate the pre-calculated plans
optimization/ParetoOptimization/tools/matRad_generateDummyPoints.m
Outdated
Show resolved
Hide resolved
Pareto optimization now supports the preprocessing function and renamed to initOptimization
Actually required for plotting functions! (Forgot to include in commit)
Updates remove some errors I had with using them in the Pareto GUI. Do not interfere with the actual functionality as far as I can tell
Added calc for variable RBE. No scenario support though
# Conflicts: # matRad/MatRad_Config.m # matRad/matRad_fluenceOptimization.m # matRad/optimization/@matRad_OptimizationProblem/matRad_objectiveFunctions.m # matRad/plotting/matRad_plotPenaltyPoints.m # matRad/util/matRad_calcCubes.m
…into dev_ParetoNav
This PR was automatically marked as stale it has been open 30 days with no activity. Please review/update/merge this PR. |
Added support for Pareto optimization and Pareto navigation