-
Notifications
You must be signed in to change notification settings - Fork 42
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
Implementation of column generation stages #525
Conversation
Codecov Report
@@ Coverage Diff @@
## release-0.4.0 #525 +/- ##
=================================================
- Coverage 83.95% 83.82% -0.14%
=================================================
Files 47 49 +2
Lines 4606 4648 +42
=================================================
+ Hits 3867 3896 +29
- Misses 739 752 +13
Continue to review full report at Codecov.
|
I have pushed the changes requested by Guillaume. In the future, the user should provide himself the primal and dual bounds in the callback. I will create an issue for that. |
I have simplified |
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'm ok for the new algorithm that call either PricingCallback or the MOI optimzer. However, I'm afraid that the MOI optimizer is always provided even if the formulation is not entered by the user.
@@ -0,0 +1,25 @@ | |||
@with_kw struct DefaultPricing <: AbstractOptimizationAlgorithm |
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 algorithm should be renamed because it can be used to solve a Formulation outside the column generation algorithm. When we don't know if the optimizer of a formulation is a pricing callback User optimizer or a MOI optimizer, we'll have to call this method to optimize the formulation (that's why SolveIpForm was supporting both the pricing callbackuser optimizer and the MOI optimizer). The name of the algorithm should state that it's going to solve the formulation.
By the way, docstring is missing.
EDIT : wording
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 do not completely understand why this algorithm can be called outside column generation. It uses pricing callback. What is the other use case?
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 is more than a pricing callback, it's the solver of the subproblem. Pricing callback is a use case of this solver.
BD.specify!.(subproblems, lower_multiplicity = 0, solver = my_pricing_callback)
PS: I edited my first comment.
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.
So, which name do you suggest?
@@ -471,7 +475,12 @@ function buildformulations!( | |||
assign_orig_vars_constrs!(spform, origform, env, annotations, ann) | |||
create_side_vars_constrs!(spform, origform, env, annotations) | |||
closefillmode!(getcoefmatrix(spform)) | |||
initialize_optimizer!(spform, getoptbuilder(prob, ann)) | |||
initialize_moioptimizer!(spform, getmoioptbuilder(prob, ann)) |
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.
If the user can use both the pricing callback and the MOI optimizer, what happens if the user didn't specify the constraints of the subproblems ? Are we going to loose time updating the solver buffer that we will never flush ?
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 did not think about this. Is it a big overhead if we have MOI optimizer for the subproblem we never use?
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 don't know but we should avoid updating the MOI optimizer if the user does not provide the whole formulation.
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 do not know how to do it.
@@ -3,7 +3,8 @@ mutable struct Formulation{Duty <: AbstractFormDuty} <: AbstractFormulation | |||
var_counter::Int | |||
constr_counter::Int | |||
parent_formulation::Union{AbstractFormulation, Nothing} # master for sp, reformulation for master | |||
optimizer::AbstractOptimizer | |||
moioptimizer::AbstractOptimizer |
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.
moioptimizer::AbstractOptimizer | |
moioptimizer::Union{Nothing, MoiOptimizer} |
to make sure you receive a MoiOptimizer
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 cannot do that because MoiOptimizer
is defined later in optimizerwrappers.jl. And I cannot move optimizerwrappers.jl above formulation.jl, as Formulation
is used there.
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.
Ok I see. That's a problem.
@@ -3,7 +3,8 @@ mutable struct Formulation{Duty <: AbstractFormDuty} <: AbstractFormulation | |||
var_counter::Int | |||
constr_counter::Int | |||
parent_formulation::Union{AbstractFormulation, Nothing} # master for sp, reformulation for master | |||
optimizer::AbstractOptimizer | |||
moioptimizer::AbstractOptimizer | |||
useroptimizer::AbstractOptimizer |
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.
useroptimizer::AbstractOptimizer | |
useroptimizer::Union{Nothing, UserOptimizer} |
to make sure you receive a UserOptimizer
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.
Same here
Guillaume, may be you implement yourself the changes you want to do? There should remain few things to do. |
Hi @rrsadykov , I just started looking at this version. The interface is really much more intuitive! Congratulations! However, there were two features in the original version that are missing here, which I consider very important: showing the current pricing stage in the log file, and counting the number of of calls to the exact stage in the test. The former helps to check whether the performance of the solver is as expected when solving larger instances, and the latter is to make sure that the heuristic pricing is effective to save some exact pricing calls. For example, the heuristic pricing might never be called or might never produce useful columns or such columns might never be inserted in the master and the test would pass (relying only in the exact phase to solve the relaxations). |
I suggest to keep the name We can change the BD.specify!.(subproblems, lower_multiplicity = 0, solver = [MOI.Gurobi, my_callback_stage2, my_callback_stage3]) and we change the stages = [
ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpForm(solver = 1)),
ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpForm(solver = 2)),
ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpForm(solver = 3)),
] So in that case, we do the exact phase with I don't see any better solution at the moment to handle #525 (comment). Moreover, if we don't fix this, we'll have a design issue with customized optimizers. I would like to have these optimizers connected to Coluna through MOI. So to avoid buffering changes several times, I suggest to identify the solvers that have the same MOI optimizers and set the parameters just before optimizing the formulation. For instance, if you define solvers of your subproblem like this, solvers = [
with_attributes(MOI.RCSP, mode = :exact),
with_attributes(MOI.RCSP, mode = :heuristic),
with_attributes(MOI.RCSP, mode = :speed_heuristic)
] Coluna will instanciate What do you think ? (cc @vitornesello ) |
Should the user also write a callback for the Is it possible here to give attributes like |
No the user doesn't write a callback when he uses a MOI optimizer. He can pass an optimizer with attributes (already working). |
If the solver in BD.specify!.(subproblems, lower_multiplicity = 0, user_solvers = [my_callback_stage2, my_callback_stage3]) and then stages = [ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpFormWithMOI(moi=MOI.Gurobi)),
ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpFormWithUserSolver(solver = 1)),
ClA.ColumnGeneration(pricing_prob_solve_alg = ClA.SolveIpFormWithUserSolver(solver = 2))
] @guimarqu, what do you think? |
Also, I do not understand how this would allow us to avoid the problem raised in #525 (comment). How Coluna knows whether to create |
We can keep the split even if the user won't call these two algorithms. Your solution won't work if one wants to optimize subproblems with different solvers. If the solver builder stored in the annotation of the subproblem returns a MOI optimizer, Coluna will create a MoiOptimizer. Otherwise it's a UserOptimizer. |
I do not think this is good idea. First, custom optimizer is not necessarily a mathematical optimizaton algorithm to be connected through MOI (Mathematical Optimization Interface). For example, RCSP solver is not a mathematical optimization solver. Second, it does not suffice to write We really like the user interface we have in VrpSolver (through mapping). For me, Coluna should be able to allow dependent packages to use any user interface they want to define subproblems (through custom annotations?). |
Yes I agree. I was speaking only about the communication between Coluna and the solver. For instance, when Coluna changes the cost of the subproblem variables to give the reduced costs to the subsolver or also the bounds of the variables. |
I think Coluna just needs to calculate the reduced costs (and bounds, etc) of the representative variables in the master ( I think that |
Current pricing stage is shown in
Counting the number of calls can easily be counted in the callback. |
If you agree with this future change #525 (comment), I think it's ready for merge. |
Ok, custom models can be given with |
Ok for me. |
* dev branch to prepare release of 0.4.0 * Move storage & records in ColunaBase (#507) * Renaming in storage (#509) * RecordContainer -> RecordWrapper * state -> record * rename lot of things, remove getters of Storage not used by Algorithms * Storage -> StorageUnitWrapper * Deletion of `AbstractData` (#510) * RecordContainer -> RecordWrapper * state -> record * rename lot of things, remove getters of Storage not used by Algorithms * start removing AbstractData structs * tests ok * getunit -> getstorageunit; StorageDict -> Storage * fix docstring * Bijection StorageUnit -> Record (#518) * Bijection StorageUnit -> Record * address Ruslan's comments * Implementation of column generation stages (#525) * Implementation of column generation stages (for example, heuristic and exact stage) * Update after conversation with Guillaume + stabilization correction * Simplification for ColCutGenConquer * Some more modifs due to Guillaume comments * Counting the number of exact calls when testing the pricing stages (#530) * Add bound callback tests (#532) * add bound callback tests * include bound callback in runtests * fix test * Apply suggestions from code review Co-authored-by: Guillaume Marques <[email protected]> * add comment * Apply suggestions from code review Co-authored-by: Guillaume Marques <[email protected]> Co-authored-by: Guillaume Marques <[email protected]> * Vector of optimizers in `Formulation` (#534) * vector of optimizers in formulation * solver_id -> optimizer_id * add Manifest * update Manifest * remove Manifest because does not work * changes * rm files * address Ruslan's comment * Update src/MathProg/optimizerwrappers.jl Co-authored-by: Vitor Nesello <[email protected]> * add Manifest * change ci * remove ci change * rm Manifest Co-authored-by: Vitor Nesello <[email protected]> * UnitsUsageDict -> UnitsUsage (#522) * UnitsUsageDict -> UnitsAccess * wip * improve * tests ok * Custom data for variables and constraints (#495) * draft for support of customer data * custom data in solution * custom data for cut callback * computecoeff * store custom data of solutions in manager * add Manifest * rm Manifest * Support to custom cuts over custom data assigned to columns with new test * tests ok Co-authored-by: Artur Alves Pessoa <[email protected]> * Prototyping custom model/optimizer (#535) * Start example * wip draft * continue * add map * works with caching optimizer * varids in Env * wrong result * multiply costs by -1 * fix scaling * Apply suggestions from code review * Update test/interfaces/model.jl Co-authored-by: Lara Pontes <[email protected]> * Follow up of "custom data" (#538) * add AbstractCustomData and set/get inc_val * fix bugs * remove duplicate methods * delete unnecessary prefixes and fix some bugs * revert some changes and update docstring * custom information for dw sp (#542) * docstring for restricted master heuristic (#543) * docstring for restricted master heuristic * Update src/Algorithm/conquer.jl * Refactoring `ObjValues` & `OptimizationState` (#544) * clean + doc * move doc from objvalues to optstate; tests of objvalues; update ci * update * update branching priorioty deprecated method * tests ok * tests ok * tests ok * delete duplicated tests Co-authored-by: Ruslan Sadykov <[email protected]> Co-authored-by: Artur Pessoa <[email protected]> Co-authored-by: Lara di Cavalcanti Pontes <[email protected]> Co-authored-by: Vitor Nesello <[email protected]> Co-authored-by: Lara Pontes <[email protected]>
Reimplements pull request #519 according to the discussion there