-
Notifications
You must be signed in to change notification settings - Fork 43
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
Broken MIP support #550
Comments
Hi, Thank you very much for this detailed description of the bug.
I agree it should work for both cases.
We'll try to do better.
If we take a look at the code of Coluna.jl/src/MathProg/solutions.jl Lines 21 to 28 in 3ef9f73
I think your example doesn't work because of the two lines that should be uncommented.
This assumption indeed applies everywhere. However, we consider a solution as integer if all integer variables have integer values. So when you are solving a LP, all solutions should be considered as integer.
Only on generated columns and pure master variables (original variables that belong to the master).
There is still a problem. In the restricted master heuristic, we should not enforce the integrality of columns from subproblems that contain only continuous variables.
I identified three issues :
We won't change the name but definitely improve docstrings or add comments.
How would you do that ? Can I put your example in the tests to fix the bugs ? |
Hi,
Correct but only in the solution returned by the restricted master heuristic. It does not prevent the column generation from finding a feasible solution that uses combination of columns because we check feasibility of the aggregated solution. It means that we project the master solution to the original formulation. Therefore, if the master solution (even with fractional columns) leads to a feasible solution in the original formulation (where all integer variables are integral), then the master solution will be considered as feasible.
It is.
Yes that was the suggestion of @rrsadykov. The easy way to implement this mechanism is to create a new constraint for each original variable in the master formulation but I'm afraid that it may decrease performances. As far as I remember, the addition of a column in the master formulation is a bottleneck so we can't afford doing this operation twice. The second solution that I though this morning is to generate theses constraints in the restricted master algorithm and adding them only in the subsolver. I think that's the way to go.
Yes this is not a bug anymore more a matter of performance / effectiveness.
Yes it is.
Ok, it's a good idea. |
I reopen beause #551 does not address the whole issue. |
Short Description
Coluna's MIP support is broken. If you submit a model with continuous variables, Coluna in many cases gives a wrong solution, or labels the problem as infeasible (despite a valid solution existing).
Environment
Coluna: Version 0.3.9 | 2021-05-15 | https://github.com/atoptima/Coluna.jl
OS: Windows 10
Example and Details
The following file contains an example that shows how coluna handles problems with continuous variables.
issue.txt
In the example, a simple min cost flow problem is setup that has an obvious solution. The problem is setup and solved in the function
solveFlowModel(f1isInteger, coluna)
, which takes 2 parameters. Iff1isInteger=true
then it sets up a MIP model where one variable (f[1]) must be integer, iff1isInteger=false
then all variables are continuous. Of course in the latter case one could simply use the Simplex method, but the model of Coluna should be general enough to handle MIP and purely continuous problems. Also it helps to analyse and understand the issue. The second parametercoluna
is the coluna solver that the model shall use. The file provides two methods to setup the solver, firstcolunaOptimizerForTreeSearch(useModifiedHeuristic)
that uses the TreeSearchAlgorithm. The conquer algorithm in TreeSearch (ColCutGenConquer
) uses a heuristic. IfuseModifiedHeuristic=false
, then the default heuristic is used (SolveIpForm
with enforcing integrality), ifuseModifiedHeuristic=true
it uses theSolveIpForm
as heuristic without enforcing integrality. The second method to setup the solver iscolunaOptimizerForColumnGeneration
, which uses the ColumnGeneration algorithm directly.The example min cost flow problem contains 3 flow variables which we call f[1], f[2] and f[3]. In the Julia-code they actually are defined as f[1,1], f[2,1], f[3,1], where the second dimension is the axis with only one entry. The latter is done so that the BlockDecomposition creates a subproblem for pricing. The resulting setup is that the subproblem/pricing problem is the min-cost-flow problem without the capacity constraints and the master problem keeps the capacity constraints.
Purely continuous Model
Let us first consider the purely continuous model. In this situation the correct solution is f[1] = 8.5, f[2] = f[3] = 1.6 and cost = 160.
Standard tree search
Using the tree search with the default heuristic gives the following, invalid solution.
Observe that coluna reports that this is an optimal solution, which it in fact is not. So what happened? I debuged into the code to analyse the issue. All following and later observations made by me in the code are based on my, quite limited, understanding of the Coluna code. It might therefore be that the analysis is incomplete or I might have understood things wrong (more comments in the code would help). When debugging into the code, it showed that the column generation algorithm generates 2 columns in 3 iterations. The first column is f[1,1] = 10.1, f[2,1] = f[3,1] = 0, the second one is f[1,1] = 0, f[2,1] = f[3,1] = 10.1. The correct solution therfore corresponds to (8.5/10.1)*first_column + (1.6/10.1)*second_column. This is exactly what the column generation algorithm also finds. However, this solution is non-integer. The issue seems to be that in the column generation algorithm a valid solution for the model needs to be completely integer. This oberservation might be also true throughout Coluna, but I am not sure. In the code a "valid problem solution" is called
ip_primal_sol
or variations of that. This indicates the assumption of a integer solution, but it is unclear, at least to me, whether this assumption applies everywhere in the code. In the column generation algorithm the assumption that a valid solution must be purely integer is explicit in line 663 in colgen.jl:The code explicitly checks for whether the solution is a purely integer solution. As this is not the case
for our problem, the column generation will finish without a solution to return. This can be seen by simply running the column generation algorithm directly:
So how did we end up with the wrong solution above if column generation does not return a solution? Well, the conquer algorithm runs a heuristic after the column generation. That heuristic uses SolveIpForm with enforcing integrality. But it runs that not just on the original model, but on the generated columns as well. It is unclear to me whether SolveIpForm works on both the generated columns and the model variables or only on the columns, but that is not important. The important thing is that the solver then looks for a single column that provides a feasible solution to the problem (single due to integrality constraint). As the first column does not provide a feasible solution to the problem (the arc for f[1] has capacity 8.5), only the second column is feasible (no capacity on the other arcs). Thus the second column provides a feasible solution, and is thus returned and falsely labeled as optimal solution.
If one introduces a capacity of 5 on the arc for f[2], then no single column is by itself feasible and Coluna would return that the problem is infeasible and the flows would be NaN.
Standard tree search with heuristic not enforcing integrality
Ok, so if the integrality constraint on the columns in the SolveIpForm heuristic leads to the wrong solution, why not just remove it? Let us see:
Yeah, that works (within some floating point precision). The SolveIpForm now can correctly return a linear combination of the two columns. Ok fine, but what if we now introduce integer variables (remember, so far all were continuous).
MIP Model (f[1] is integer)
Here the correct solution is f[1] = 8, f[2] = f[3] = 2.1 and cost = 210.
Standard tree search
Well, let's see:
Again, we get the wrong solution, in fact exactly the same solution as in the continuous case. The column generation here also returns 2 columns. The first is f[1] = 10, f[2] = f[3] = 0.1 and the second is f[1] = 0, f[2] = f[3] = 10.1. The first column is slightly different due to the integrality constraint on f[1], but the second column is exactly the same as in the purely continuous model. The problem described in the purely continuous case is also occuring here, the column generation returns no solution and the final SolveIpForm returns the only feasible single column as wrong optimal solution.
Well, that was expected after what we observed in the purely continuous case, so why not just solve it by removing the integrality constraint on the heuristic (SolveIpForm)?
Standard tree search with heuristic not enforcing integrality
Well, the heuristic returned exactly the same solution as in the purely continuous case, with non-integer flow in f[1]. This is due to that we removed any integer requirement in the heuristic, and the column generation returned no solution.
Consequences
The support for continuous variables needs to be rectified in Coluna. At least the column generation algorithm needs to be modified to remove the assumption of purely integer solutions. It should also be checked whether this assumption exists in other places in Coluna (the check for
isinteger
exists also other places, e.g. solvelpform.jl, branching and benders.jl).Maybe one can modify the name from "ip_primal_sol" to "mip_primal_sol" or "feasible_primal_problem_sol" or similar to explicitly show that it might not be an ip (purely integer) solution.
Moreover one needs to ensure that Coluna does not return suboptimal solutions that are flagged as optimal, labels the problem wrongly as infeasible, or returns solutions where integer variables have non-integer values.
The text was updated successfully, but these errors were encountered: