Cbc icon indicating copy to clipboard operation
Cbc copied to clipboard

SOS constraints on continuous variables

Open lovasoa opened this issue 3 years ago • 16 comments

I am trying to solve the following "toy" problem using the C API of cbc

Maximize
  x + y

Bounds
  x <= 2
  y <= 3

SOS
  sos1: S1 :: x : 1  y : 2
End

all variables are continuous. The expected solution is x=0.0 and y=3.0. But cbc returns x=2.0 and y=3.0.

No error or warning is raised, but cbc seems to silently ignore the SOS constraint.

lovasoa avatar Mar 19 '21 16:03 lovasoa

If I run that as a .lp file using standalone cbc (2.10.5), I get an error message.

To get it to work, I added

Subject To

and it gave me the correct result.

On 19/03/2021 16:10, Ophir LOJKINE wrote:

I am trying to solve the following "toy" problem using the C API of cbc

|Maximize x + y Bounds x <= 2 y <= 3 SOS sos1: S1 :: x : 1 y : 2 End |

all variables are continuous. The expected solution is |x=0.0| and |y=3.0|. But cbc returns |x=2.0| and |y=3.0| (ignoring the SOS constraint).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/coin-or/Cbc/issues/376, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHCK6P6ST7HTNW5IJKTTENZO7ANCNFSM4ZPCRSZA.

jjhforrest avatar Mar 19 '21 17:03 jjhforrest

I am using the C API (through the rust binding), not the LP format parser.

lovasoa avatar Mar 19 '21 17:03 lovasoa

You can reproduce the issue for instance by running this test in the rust binding :

https://github.com/KardinalAI/coin_cbc/blob/master/src/raw.rs#L527-L550

If you remove these lines:

        m.set_integer(0);
        m.set_integer(1);

then the solver returns [-1, -1] instead of [-1, 0]

lovasoa avatar Mar 19 '21 18:03 lovasoa

Trying to debug that further, I see that the status of the problem after the solve is 0 when the variables are integers, but -1 when they are continuous. Is that related ?

lovasoa avatar Mar 19 '21 18:03 lovasoa

Adding an integer variable with an objective coefficient of 0 and a no constraint seems to be enough to force Cbc to take the SOS constraint into account even for the other, non-integer variables.

@jjhforrest : could this be a bug where the branching algorithm is not started if the problem doesn't contain at least one integer variable ?

lovasoa avatar Mar 19 '21 18:03 lovasoa

Testing the workaround, it seems to work on macos, but on linux, cbc fails with

CglPreProcess.cpp:2492: OsiSolverInterface* CglPreProcess::preProcessNonDefault(OsiSolverInterface&, int, int, int): Assertion `numberProhibited_ == oldModel->getNumCols()' failed.

should I open a separate issue ?

lovasoa avatar Mar 19 '21 19:03 lovasoa

Related issue: https://github.com/coin-or/Cbc/issues/363

odow avatar Mar 19 '21 20:03 odow

@jjhforrest : could this be a bug where the branching algorithm is not started if the problem doesn't contain at least one integer variable ?

I don't think it is a bug in cbc - may be a bug in rust interface. cbc solved with .lp file quite happily.

jjhforrest avatar Mar 20 '21 17:03 jjhforrest

I don't think it is a bug in cbc - may be a bug in rust interface. cbc solved with .lp file quite happily.

I think this is a bug in the cbc C interface. The rust interface just calls Cbc_solve. The bug is that Cbc_solve doesn't take into account the SOS constraints when there is no integer variable.

lovasoa avatar Mar 20 '21 21:03 lovasoa

@jjhforrest

Here is a C program that illustrates that the issue is not in the rust binding but in cbc itself:

 #include  <coin/Cbc_C_Interface.h>
 #include  <stdio.h>
 #include  <string.h>

 int  main(int  argc,char* argv[])
 {
      Cbc_Model* m =Cbc_newModel();
      int  numcols = 2;
      int  numrows = 0;
      int  start[3] = {0};
      int  *index  = 0;
      double  *value =0;
      double  *collb = 0;
      double  colub[] = {2.0, 3.0};
      double  obj[] = {1.0, 1.0};
      double  *rowlb =0;
      double  *rowub =0;
      Cbc_loadProblem(m,
          numcols, numrows,
          start,index,
          value,
          collb, colub,
          obj,
          rowlb, rowub);
      Cbc_setObjSense(m, -1); // maximize

      // sos: x:1 y:2
      int sosRowStarts[] = {0, 2};
      int sosColIndices[] = {0, 1};
      double sosWeights[] = {1., 2.};
      int sosType = 1;
      Cbc_addSOS(m, 1, sosRowStarts, sosColIndices, sosWeights, sosType);

      Cbc_writeLp(m, "/tmp/test.lp");

      int status = Cbc_solve(m);
      printf("Solve status: %d\n", status);
      const double* solution = Cbc_getColSolution(m);
      printf("solution: [%f, %f]", solution[0], solution[1]); 
      Cbc_deleteModel(m);
      return  0;
 }

lovasoa avatar Mar 20 '21 21:03 lovasoa

There may be problems in C interface but your C program is incorrect (and segfaults when I run it).

There are two columns so it should be

int start[3] = {0};

as the code thinks the total number of elements is start[numcols]

The program then gets the correct answer. I do not understand why some messages occur as I am not familiar with C interface.

On 20/03/2021 21:54, Ophir LOJKINE wrote:

@jjhforrest https://github.com/jjhforrest

Here is a C program that illustrates that the issue is not in the rust binding but in cbc itself:

#include <coin/Cbc_C_Interface.h> #include <stdio.h> #include <string.h>

int main(int argc,char* argv[]) { Cbc_Model* m =Cbc_newModel(); int numcols =2; int numrows =0; int start[2] = {0}; int *index =0; double *value =0; double *collb =0; double colub[] = {2.0,3.0}; double obj[] = {1.0,1.0}; double *rowlb =0; double *rowub =0; Cbc_loadProblem(m, numcols, numrows, start,index, value, collb, colub, obj, rowlb, rowub); Cbc_setObjSense(m, -1);// maximize

   // sos: x:1 y:2
   int  sosRowStarts[] = {0,2};
   int  sosColIndices[] = {0,1};
   double  sosWeights[] = {1.,2.};
   int  sosType =1;
   Cbc_addSOS(m,1, sosRowStarts, sosColIndices, sosWeights, sosType);

   Cbc_writeLp(m,"/tmp/test.lp");

   int  status =Cbc_solve(m);
   printf("Solve status: %d\n", status);
   const  double* solution =Cbc_getColSolution(m);
   printf("solution: [%f, %f]", solution[0], solution[1]);
   Cbc_deleteModel(m);
   return   0;

}

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/coin-or/Cbc/issues/376#issuecomment-803468863, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHFT7XHVTEOUS5X3S6DTEUKRPANCNFSM4ZPCRSZA.

jjhforrest avatar Mar 21 '21 11:03 jjhforrest

You are right, I updated the code above. It doesn't segfault, and illustrates that the bug is in cbc, not the rust interface.

lovasoa avatar Mar 21 '21 12:03 lovasoa

I can take a look at this today (I think I know what the issue is). Until last week, JuMP was using a patched version of Cbc, which was why we didn't find this earlier.

odow avatar Mar 21 '21 18:03 odow

This is fixed on master https://github.com/coin-or/Cbc/blob/1706330116dfbc2e474790963506cc00dbf2ead8/src/Cbc_C_Interface.cpp#L2146 whereas the 2.10 branch doesn't check for SOS constraints: https://github.com/coin-or/Cbc/blob/f40e4104b9f29d968ec3181fc5e534af31c0717f/Cbc/src/Cbc_C_Interface.cpp#L848

What is the status of the 3.0 release?

odow avatar Mar 21 '21 21:03 odow

What is the status of the 3.0 release?

There is a draft PR open (#374) that I'd like to recruit some help in finishing before we go to 3.0. But I know the natives are getting restless. What's left do at a minimum is

  • some debugging (there are still probably a few surface-level bugs hanging around that I haven't had time to track down) and
  • some thorough testing to make sure there are no performance regressions.

I would also really like to create a minimal CbcSolver class, rename CbcMain1() and move it into that class, make CbcMain0() the default constructor of the CbcParameters class, and finally break up the humongous while loop into some smaller pieces so we can reformat the code. Currently, clang-reformat can't even parse the code in CbcSolver.cpp. Emacs also gets lost when trying to find matching parentheses. It would be a great test case for a code reformatter! I will post a message to Discussions about this. None of this would change any functionality and shouldn't take much time. The heavy lifting is done (there are some nice-to-have things listed in the PR that are a little more difficult, but we could push those off to another release if needed).

tkralphs avatar Mar 22 '21 01:03 tkralphs

Has this been fixed?

ValentinKaisermayer avatar May 03 '24 07:05 ValentinKaisermayer