Skip to content
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

Added support to pricing phases for use of heuristic pricing #519

Closed
wants to merge 3 commits into from

Conversation

artalvpes
Copy link
Collaborator

No description provided.

@artalvpes
Copy link
Collaborator Author

I tested both CVRP and VRPTW applications with the branch testing_heuristic_pricing of ColunaBenchmarks and fast_testing_run of Camina.

@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #519 (2cd62a3) into release-0.4.0 (b5795cb) will increase coverage by 0.04%.
The diff coverage is 91.17%.

Impacted file tree graph

@@                Coverage Diff                @@
##           release-0.4.0     #519      +/-   ##
=================================================
+ Coverage          83.95%   84.00%   +0.04%     
=================================================
  Files                 47       47              
  Lines               4606     4619      +13     
=================================================
+ Hits                3867     3880      +13     
  Misses               739      739              
Impacted Files Coverage Δ
src/Algorithm/colgen.jl 96.56% <88.88%> (+0.08%) ⬆️
src/Algorithm/basic/solveipform.jl 92.77% <100.00%> (ø)
src/Algorithm/branching/branchinggroup.jl 89.24% <100.00%> (ø)
src/MathProg/optimizerwrappers.jl 94.44% <100.00%> (+0.24%) ⬆️
src/decomposition.jl 99.11% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5795cb...2cd62a3. Read the comment docs.

@rrsadykov
Copy link
Collaborator

Dear Artur,

Sorry if this was not clear, but the code architecture supposes that col.gen. phases should be implemented at the level of ColCutGenConquer. This means that if there are two phases, then ColCutGenConquer calls Column Generation two times. This is much more flexible as it allow us to do something else between col.gen. phases (for example generating rounded capacity cuts for the CVRP before going to the exact pricing). Then this would allow even to switch algorithms between phases. For example, using subgradient (or volume) algorithm with heuristic pricing and column generation with exact pricing.

I suggest to replace colgen field in ColCutGenConquer by phases, which would be a vector of AbstractOptimizationAlgorithm. Then ColCutGenConquer would call algorithms in phases one by one. Also, ColCutGenConquer would manage updating the bounds (for example node dual bound would be updated only with col.gen. bound obtained in the last phase). Thus ColumnGeneration does not even need to know whether the current phase is exact or not. In the future, we may have a more complex interaction between column generation phases and cut separation algorithms (than just calling all cut separators after all col. gen. phases).

I would also suggest to "extract" pricing callback from SolveIpForm algorithm, and to define a separate algorithm for the pricing callback. Col.gen. phase number will a parameter of this callback algorithm. For SolveIpForm algorithm, phase does not make any sense as the formulation is always solved to optimality (at least currently). If we have separated algorithms for solving MIP and for the user pricing callback, then for the heuristic phase pricing_prob_solve_alg would be set to the pricing callback algorithm (with the phase number correctly set), and for the exact phase pricing_prob_solve_alg would be set to SolveIpForm as now.

We may have a video call with you and Guillaume to discuss this.

Copy link
Collaborator

@rrsadykov rrsadykov left a comment

Choose a reason for hiding this comment

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

See my previous comment

@rrsadykov
Copy link
Collaborator

Also, the name phases is to discuss, as we have already phases in strong branching. May be should use another term, or just be more precise: conquer_phases

@@ -60,12 +60,11 @@ end

function product_score(group::BranchingGroup, parent_optstate::OptimizationState)
parent_inc = getincumbents(parent_optstate)
# TO DO : we need to mesure the gap to the cut-off value
Copy link
Contributor

Choose a reason for hiding this comment

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

why this comment has been removed ?

Comment on lines +47 to +49
function setphase!(opt::MoiOptimizer, ph::Int)
return
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function setphase!(opt::MoiOptimizer, ph::Int)
return
end
setphase!(opt::MoiOptimizer, ph::Int) = nothing

Comment on lines +20 to 23
function setphase!(opt::UserOptimizer, ph::Int)
opt.phase = ph
return
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function setphase!(opt::UserOptimizer, ph::Int)
opt.phase = ph
return
end
setphase!(opt::UserOptimizer, ph::Int) = opt.phase = ph

@@ -427,7 +427,7 @@ end

function getoptbuilder(prob::Problem, ann::BD.Annotation)
if BD.getpricingoracle(ann) !== nothing
return () -> UserOptimizer(BD.getpricingoracle(ann))
return () -> UserOptimizer(BD.getpricingoracle(ann), 0) # exact phase
Copy link
Contributor

Choose a reason for hiding this comment

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

It it's always 0 by default it should be in the constructor ?

@guimarqu
Copy link
Contributor

guimarqu commented May 3, 2021

Sorry if this was not clear, but the code architecture supposes that col.gen. phases should be implemented at the level of ColCutGenConquer. This means that if there are two phases, then ColCutGenConquer calls Column Generation two times. This is much more flexible as it allow us to do something else between col.gen. phases (for example generating rounded capacity cuts for the CVRP before going to the exact pricing). Then this would allow even to switch algorithms between phases. For example, using subgradient (or volume) algorithm with heuristic pricing and column generation with exact pricing.

I suggest to replace colgen field in ColCutGenConquer by phases, which would be a vector of AbstractOptimizationAlgorithm. Then ColCutGenConquer would call algorithms in phases one by one. Also, ColCutGenConquer would manage updating the bounds (for example node dual bound would be updated only with col.gen. bound obtained in the last phase). Thus ColumnGeneration does not even need to know whether the current phase is exact or not. In the future, we may have a more complex interaction between column generation phases and cut separation algorithms (than just calling all cut separators after all col. gen. phases).

This is indeed the way to go.

I would also suggest to "extract" pricing callback from SolveIpForm algorithm, and to define a separate algorithm for the pricing callback. Col.gen. phase number will a parameter of this callback algorithm. For SolveIpForm algorithm, phase does not make any sense as the formulation is always solved to optimality (at least currently). If we have separated algorithms for solving MIP and for the user pricing callback, then for the heuristic phase pricing_prob_solve_alg would be set to the pricing callback algorithm (with the phase number correctly set), and for the exact phase pricing_prob_solve_alg would be set to SolveIpForm as now.

I couldn't extract the pricing callback from SolveIpForm because SolveIpForm knows if its a callback or a MOI.optimize call at execution time. Colgen does not know how the subproblems are solved because the model is unknown when the user parametrizes the algorithms. Therefore, the information must pass through BlockDecomposition & MathProg. When we retrieve how the user optimizes the subproblems , we wrap the pricing in an UserOptimizer <: AbstractOptimizer or the mip optimizer in MoiOptimizer <: AbstractOptimizer and we use dispatch in SolveIpForm.

I think the best alternative is to rename SolveIpForm.

We may have a video call with you and Guillaume to discuss this.

Since I did not make a lot of progress last two weeks, we can discuss this Thursday. The meeting is at 11am France which is very early in Brazil, we can do it later if you want.

@rrsadykov
Copy link
Collaborator

I couldn't extract the pricing callback from SolveIpForm because SolveIpForm knows if its a callback or a MOI.optimize call at execution time. Colgen does not know how the subproblems are solved because the model is unknown when the user parametrizes the algorithms. Therefore, the information must pass through BlockDecomposition & MathProg. When we retrieve how the user optimizes the subproblems , we wrap the pricing in an UserOptimizer <: AbstractOptimizer or the mip optimizer in MoiOptimizer <: AbstractOptimizer and we use dispatch in SolveIpForm.

Is it possible to force solving the subproblem MIP even if the pricing callback is defined? If yes, that what we can do is to create two separate algorithms for solving MIP (current SolveIpForm) and for the pricing callback. The latter will trigger an error if it sees in runtime that no callback is defined. SolveIpForm would force to solve the MIP even if a callback is defined.

@guimarqu
Copy link
Contributor

guimarqu commented May 4, 2021

Is it possible to force solving the subproblem MIP even if the pricing callback is defined? If yes, that what we can do is to create two separate algorithms for solving MIP (current SolveIpForm) and for the pricing callback. The latter will trigger an error if it sees in runtime that no callback is defined. SolveIpForm would force to solve the MIP even if a callback is defined.

How column generation will know what algorithm it should run to optimize the subproblems ?

@rrsadykov
Copy link
Collaborator

How column generation will know what algorithm it should run to optimize the subproblems ?

There is the field pricing_prob_solve_alg in ColumnGeneration for that.

@guimarqu
Copy link
Contributor

guimarqu commented May 4, 2021

There is the field pricing_prob_solve_alg in ColumnGeneration for that.

It means that if the user decides to solve the subproblems with a pricing callback, he must change the solver through BlockDecomposition and he must also change the pricing_prob_solve_alg field of ColumnGeneration ? Such a change will degrade the user experience.

@rrsadykov
Copy link
Collaborator

It means that if the user decides to solve the subproblems with a pricing callback, he must change the solver through BlockDecomposition and he must also change the pricing_prob_solve_alg field of ColumnGeneration ? Such a change will degrade the user experience.

I agree that it is less convenient. However, this is more flexible. For example, the user can define the heuristic callback and use it together with MIP (for the exact phase). This is impossible for the moment.

Inconvenience can be minimized by defining a shortcut BranchCutAndPriceAlgorithm for TreeSearchAlgorithm. There, we add on option to switch between solving with MIP and solving with the callback. Changing that option will be very easy. May be it will be even possible to have an option for automatic choice (see in runtime if callback is defined and use it in this case, otherwise use MIP).

@guimarqu
Copy link
Contributor

guimarqu commented May 4, 2021

I agree that it is less convenient. However, this is more flexible. For example, the user can define the heuristic callback and use it together with MIP (for the exact phase). This is impossible for the moment.

Inconvenience can be minimized by defining a shortcut BranchCutAndPriceAlgorithm for TreeSearchAlgorithm. There, we add on option to switch between solving with MIP and solving with the callback. Changing that option will be very easy. May be it will be even possible to have an option for automatic choice (see in runtime if callback is defined and use it in this case, otherwise use MIP).

I think we can split the pricing callback and the call to the MOI solver into two algorithms : PricingCallback and SolveIpForm. But we need an algorithm on top of them that will dispatch to PricingCallback or SolveIpForm. This algorithm will be the default one to not change the default behaviour of Coluna.

This solution allow us more flexibility without inconvenience.

@rrsadykov
Copy link
Collaborator

Good for me.

@artalvpes
Copy link
Collaborator Author

artalvpes commented May 4, 2021

Hello, guys. It is nice to see that this branch motivated a lot of improvements on the structure of Coluna. However, I will would like to remark that heuristic pricing is already working in the way it is implemented and it is important to accelerate the executions needed to test other new features. So, I am afraid that delaying this PR with several changes in the structure might delay the whole project. How about opening an issue for that letting these changes for another branch?

@rrsadykov
Copy link
Collaborator

Dear Artur. The problem with this pull request that it adds a number of changes to ColumnGeneration algorithm which should be reverted after. I am not sure that this is a way to go. I let Guillaume decide. If Gullaume decides to accept this pull request, some tests for heuristic pricing should be added to it so that this functionality is not lost after future changes.

@guimarqu
Copy link
Contributor

guimarqu commented May 4, 2021

@artalvpes The discussion went off topic. We won't wait for the split of SolveIpForm to merge this PR.
However, as Ruslan said, if we merge this PR, we'll have to revert a lot of changes. This is a review of how the phases should be implemented in the colgen algorithm :

  • the phase should be a parameter of the ColumnGeneration algorithm

    @with_kw struct ColumnGeneration <: AbstractOptimizationAlgorithm

  • the field colgen of ColCutGenConquer should be a vector of ColumnGeneration algorithm. If you want to use two different phases, you'll have two instance of ColumnGeneration with different phase values.

    @with_kw struct ColCutGenConquer <: AbstractConquerAlgorithm
    colgen::AbstractOptimizationAlgorithm = ColumnGeneration()
    primal_heuristics::Vector{ParameterisedHeuristic} = [DefaultRestrictedMasterHeuristic()]
    preprocess = PreprocessAlgorithm()
    cutgen = CutCallbacks()
    max_nb_cut_rounds::Int = 3 # TODO : tailing-off ?
    run_preprocessing::Bool = false
    opt_atol::Float64 = colgen.opt_atol # TODO : force this value in an init() method
    opt_rtol::Float64 = colgen.opt_rtol # TODO : force this value in an init() method
    end

  • you should loop over the vector of ColumnGeneration algorithms :

    nb_tightening_rounds = 0
    colgen_output = run!(algo.colgen, env, reform, OptimizationInput(nodestate))
    update!(nodestate, getoptstate(colgen_output))
    node_pruned_by_colgen = getterminationstatus(nodestate) == INFEASIBLE ||
    ip_gap_closed(nodestate, atol = algo.opt_atol, rtol = algo.opt_rtol)
    node_pruned = false

I'm ok with the changes done in SolveIpForm.

@rrsadykov
Copy link
Collaborator

Hi, Artur. Ok, can please add a test in this pull request, and then it will be accepted.

@artalvpes
Copy link
Collaborator Author

artalvpes commented May 6, 2021

Hi, Artur. Ok, can please add a test in this pull request, and then it will be accepted.

Hi Ruslan and Guillaume. I started to check the changes suggested by Guillaume but I have some concerns that go in the direction of the previous discussion. I think that the name phases for ColumnGeneration is already used for the classical two phases of the Simplex algorithm. So, I think that creating several copies of ColumnGeneration, one for each pricing phase, will be misleading. Note that switching between two pricing phases may occur before or after switching between the two simplex phases. Because of that, I agree with Ruslan that the pricing phase should be somehow attached to PricingCallback instead of ColumnGeneration but I agree with Guillaume that having to configure this through both BlockDecomposition and an attribute of ColumnGeneration would not be convenient.

Thus, I would like to ask Guillaume if he agrees with Ruslan to let it as is for now until we decide the best way to refactor it. Then I would only add a test and go the next task.

@artalvpes
Copy link
Collaborator Author

artalvpes commented May 6, 2021

Perhaps the problem is that we want Coluna to control something that is not its responsibility. Perhaps, the best way to implement this is letting the user switch to a more exhaustive pricing method when the current one fails in the same pricing call, just reporting to Coluna in some way that this change occurred (to reset the pseudo_dual_bound) and whether the obtained solutions give a valid dual bound or not. The user would also need to be able to save a pricing state (with the current pricing phase but Coluna does not need to know about it) to be recovered in the next callback of the same node. This is also more generic because it would allow for example a premature change in the pricing phase according to some criterion known only by the pricing solver. To allow solving the pricing subproblem by MIP in some phase, I think we could export a special function to be called inside the callback. What do you think?

@rrsadykov
Copy link
Collaborator

rrsadykov commented May 7, 2021

Dear Artur,

We talked with Guillaume yesterday and we agreed that it is better not to merge changes which will be reverted after. This is a double work.

Concerning the names, I propose to call change "phases" name for "stages". So we would have pricing stages (or more generally conquer stages) which may consist of several simplex phases.

Perhaps the problem is that we want Coluna to control something that is not its responsibility. Perhaps, the best way to implement this is letting the user switch to a more exhaustive pricing method when the current one fails in the same pricing call, just reporting to Coluna in some way that this change occurred (to reset the pseudo_dual_bound) and whether the obtained solutions give a valid dual bound or not. The user would also need to be able to save a pricing state (with the current pricing phase but Coluna does not need to know about it) to be recovered in the next callback of the same node. This is also more generic because it would allow for example a premature change in the pricing phase according to some criterion known only by the pricing solver. To allow solving the pricing subproblem by MIP in some phase, I think we could export a special function to be called inside the callback. What do you think?

I think this suggestion comes against the "design philosophy" of Coluna. The Coluna design suggests that every algorithm is independent from other algorithms. Algorithms communicate only through input and output and also through storage units. But in any case there should be no communication between algorithms except running one from another. If other type of communication is unavoidable, two communicating algorithms should be merged into one. For example, in your code, you have setphase!(spform.optimizer, pricing_phase). This is the communication between algorithms which should not be allowed, it breaks algorithms independence. Also, your suggestion to keep some information between callback calls complicates the user's life. In addition, for me, callback should be a "terminal" algorithm (a leaf in the algorithms tree). What you suggest is to call SolveIpForm algorithm by the user callback. For me, this is "dirty".

@rrsadykov
Copy link
Collaborator

What we suggest with Gullaume is pretty simple to implement for me. We should define field stages in ColGutGenConquer, which a vector of ColumnGeneration algorithms. We should also define a "dispatch" algorithm which we can call PricingAlgorithm which is of type AbstractOptimizationAlgoirithm. This algorithm will have two child algorithms: 'SolveIpForm' and PricingCallback (which should be defined and which will have stagenumber parameter). PricingAlgorithm should have parameter to choose between 1) always call callback, 2) always call MIP, and default 3) call callback if defined, otherwise MIP.

The default parameterisation of PricingAlgorithm would be

struct PricingAlgorithm <: AbstractOptimisationAlgoirithm
    MIPalg = SolveIpForm( deactivate_artificial_vars=false, enforce_integrality=false, log_level=2),
    callbackalg = PricingCallback(stagenumber=0),
    dispatch = 3 # automatic
end

The default parameterisation of ColumnGeneration would be pricing_prob_solve_alg = PricingAlgorithm(), and default parameterisation of ColCutAndGenConquer would be stages=[ColumnGeneration()]. If user want to have heuristic and exact stages, the parameterisation would be

stages=[ColumnGeneration(),
        ColumnGeneration(pricing_prob_solve_alg = PricingAlgorithm(callbackalg = PricingCallback(stagenumber=1)))]

If user wants that the exact pricing is solved by MIP, it would be

stages=[ColumnGeneration(pricing_prob_solve_alg = PricingAlgorithm(dispatch = 2)),
        ColumnGeneration(pricing_prob_solve_alg = PricingAlgorithm(callbackalg = PricingCallback(stagenumber=1)))]

Of course, dispatch modes (1, 2, 3) should be named somehow.

Parameterisation will later be simplified with an alias (BranchCutAndPriceAlgorithm)

@guimarqu
Copy link
Contributor

guimarqu commented May 7, 2021

Dear Artur,

We talked with Guillaume yesterday and we agreed that it is better not to merge changes which will be reverted after. This is a double work.

Concerning the names, I propose to call change "phases" name for "stages". So we would have pricing stages (or more generally conquer stages) which may consist of several simplex phases.

Ok for me.

Perhaps the problem is that we want Coluna to control something that is not its responsibility. Perhaps, the best way to implement this is letting the user switch to a more exhaustive pricing method when the current one fails in the same pricing call, just reporting to Coluna in some way that this change occurred (to reset the pseudo_dual_bound) and whether the obtained solutions give a valid dual bound or not.

From your previous message, I understand that we must be able to stop column generation from any point. Can we have an extra field in the pricing callback output to tell Column Generation that it should stop and let ConCutGenConquer run the next column generation algorithm ?

The user would also need to be able to save a pricing state (with the current pricing phase but Coluna does not need to know about it) to be recovered in the next callback of the same node. This is also more generic because it would allow for example a premature change in the pricing phase according to some criterion known only by the pricing solver. To allow solving the pricing subproblem by MIP in some phase, I think we could export a special function to be called inside the callback. What do you think?

I think this suggestion comes against the "design philosophy" of Coluna. The Coluna design suggests that every algorithm is independent from other algorithms. Algorithms communicate only through input and output and also through storage units. But in any case there should be no communication between algorithms except running one from another. If other type of communication is unavoidable, two communicating algorithms should be merged into one. For example, in your code, you have setphase!(spform.optimizer, pricing_phase). This is the communication between algorithms which should not be allowed, it breaks algorithms independence.

I think this one can be fixed by passing the stage as argument of the SolveIpForm.

Also, your suggestion to keep some information between callback calls complicates the user's life. In addition, for me, callback should be a "terminal" algorithm (a leaf in the algorithms tree). What you suggest is to call SolveIpForm algorithm by the user callback.

I would add that such a mechanism is "out of bounds" for callbacks because the user can keep his data in the parent scope of the pricing callback method (even if it cannot know when the node changes at the moment). The goal of the pricing callback is to allow the user to quickly implement/test a pricing solver. However, we will have such a mechanism for the custom pricing solver because it will be attached to a model that may have a storage. So, we should keep this mechanism for the custom pricing solver.

For me, this is "dirty".

It can be improved :)

@rrsadykov
Copy link
Collaborator

I can implement support of pricing stages. But I first need a test for this case.

@rrsadykov
Copy link
Collaborator

rrsadykov commented May 7, 2021

From your previous message, I understand that we must be able to stop column generation from any point. Can we have an extra field in the pricing callback output to tell Column Generation that it should stop and let ConCutGenConquer run the next column generation algorithm ?

I do not think that stopping col.gen. prematurely should be decided by pricing. The column generation algorithm should decide itself when and why it should stop.

It would be very much welcome to leave PricingAlgorithm as an AbstractOptimizationAlgorithm (so that it is easily replaceable by any other optimization algorithm) and not have any extra fields in the optimization input and optimization output.

If you do not agree with this, I would like to know the concrete case when this behaviour is needed.

@artalvpes
Copy link
Collaborator Author

From your previous message, I understand that we must be able to stop column generation from any point. Can we have an extra field in the pricing callback output to tell Column Generation that it should stop and let ConCutGenConquer run the next column generation algorithm ?

I do not think that stopping col.gen. prematurely should be decided by pricing. The column generation algorithm should decide itself when and why it should stop.

It would be very much welcome to leave PricingAlgorithm as an AbstractOptimizationAlgorithm (so that it is easily replaceable by any other optimization algorithm) and not have any extra fields in the optimization input and optimization output.

If you do not agree with this, I would like to know the concrete case when this behaviour is needed.

I was not suggesting that the pricing could be able to stop col.gen.. I was suggesting that the pricing could be able to automatically switch to the next algorithm stage when solving the pricing subproblem. However, this has an impact in Coluna because, as the quality of the columns improve, the pseudo dual cost of the generated columns may get worse (the better the column, the worse its pseudo dual cost) preventing the stability center to be updated and worsening the convergence of col. gen.

@artalvpes
Copy link
Collaborator Author

I think this suggestion comes against the "design philosophy" of Coluna. The Coluna design suggests that every algorithm is independent from other algorithms. Algorithms communicate only through input and output and also through storage units. But in any case there should be no communication between algorithms except running one from another. If other type of communication is unavoidable, two communicating algorithms should be merged into one. For example, in your code, you have setphase!(spform.optimizer, pricing_phase).

Ok. I agree with that but for me it is just a matter of using the "official" methods to send the same information (either algorithm inputs and outputs or the storage units).

This is the communication between algorithms which should not be allowed, it breaks algorithms independence. Also, your suggestion to keep some information between callback calls complicates the user's life. In addition, for me, callback should be a "terminal" algorithm (a leaf in the algorithms tree). What you suggest is to call SolveIpForm algorithm by the user callback. For me, this is "dirty".

In fact, I personally don't think that solving a pricing subproblem by MIP is efficient in practice. Specially, if one has a specific pricing algorithm in hand, being able to call it alternately with a MIP does not make much sense for me. So I tried to propose a way not to change the structure of Coluna only to allow for using this option.

@artalvpes
Copy link
Collaborator Author

I can implement support of pricing stages. But I first need a test for this case.

That's a good suggestion! If you want, I can leave this discussion making whatever you decide and concentrate in the algorithms themselves ;-)

@artalvpes
Copy link
Collaborator Author

I can implement support of pricing stages. But I first need a test for this case.

I added a test in "test/pricing_callback_tests.jl". It is using the current interface for the pricing phases. You need to update the test when the interface changes.

@rrsadykov
Copy link
Collaborator

Reimplemented in pull request #525

@rrsadykov rrsadykov closed this May 10, 2021
@guimarqu guimarqu deleted the adding_pricing_phases branch September 14, 2022 07:04
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.

3 participants