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

SOS constraints on continuous variables #376

Open
lovasoa opened this issue Mar 19, 2021 · 15 comments
Open

SOS constraints on continuous variables #376

lovasoa opened this issue Mar 19, 2021 · 15 comments

Comments

@lovasoa
Copy link

lovasoa commented Mar 19, 2021

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.

@jjhforrest
Copy link
Contributor

jjhforrest commented Mar 19, 2021 via email

@lovasoa
Copy link
Author

lovasoa commented Mar 19, 2021

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

@lovasoa
Copy link
Author

lovasoa commented Mar 19, 2021

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
Copy link
Author

lovasoa commented Mar 19, 2021

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
Copy link
Author

lovasoa commented Mar 19, 2021

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
Copy link
Author

lovasoa commented Mar 19, 2021

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 ?

@odow
Copy link

odow commented Mar 19, 2021

Related issue: #363

@jjhforrest
Copy link
Contributor

@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.

@lovasoa
Copy link
Author

lovasoa commented Mar 20, 2021

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
Copy link
Author

lovasoa commented Mar 20, 2021

@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;
 }

@jjhforrest
Copy link
Contributor

jjhforrest commented Mar 21, 2021 via email

@lovasoa
Copy link
Author

lovasoa commented Mar 21, 2021

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

@odow
Copy link

odow commented Mar 21, 2021

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
Copy link

odow commented Mar 21, 2021

This is fixed on master

|| ((solver->getNumIntegers()+model->nSos)==0)) {

whereas the 2.10 branch doesn't check for SOS constraints:
if (solver->getNumIntegers() == 0 || model->relax_ == 1) {

What is the status of the 3.0 release?

@tkralphs
Copy link
Member

tkralphs commented Mar 22, 2021

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).

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

No branches or pull requests

4 participants