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

new solver: CHOCO #169

Merged
merged 9 commits into from
Aug 7, 2019
Merged

new solver: CHOCO #169

merged 9 commits into from
Aug 7, 2019

Conversation

pchtsp
Copy link
Collaborator

@pchtsp pchtsp commented Mar 27, 2018

Good day,

The open source CHOCO solver recently released an MPS integration for input data. I wanted to test my model an came up with a lightweight integration for this solver in pulp.

I think it's nice to add a CP solver to the mix of pulp's solvers. Specially since it's open source.

It re-uses the other CMD solvers' structure. Specifically I took into account the GUROBI_CMD, CPLEX_CMD and COIN_CMD classes.

It's built over the PR I made to get solving and solution status separately. Because choco also makes a distinction between 'finding the optimal' and 'finding a solution'. Having said that, if the other change is not considered important enough or desirable to pulp I could include this solver on top of the master branch directly without this distinction.

The solver is built in java so it needs java installed to work, which I care to check. For the time being, for it to work it needs to have java in the PATH and the user needs to give the absolute path to the jar binary. The binary needs to be built from sources (for now, since it's a new version) but it will be available for download soon, according to the developers.

For more information, see the following:

If you have any questions, comments or changes, I'll be glad to discuss them!

Franco

@stumitchell
Copy link
Contributor

Great news I will try and look at this over Easter and hopefully do a release

@pchtsp
Copy link
Collaborator Author

pchtsp commented Mar 28, 2018 via email

…via PULP_CHOCO_CMD just like PULP_CBC_CMD does
@pchtsp
Copy link
Collaborator Author

pchtsp commented Mar 28, 2018

Ok, changes done. I still need to understand how to configure the running of the tests to be done with the PULP_CHOCO_CMD solver too.

@stumitchell
Copy link
Contributor

you need to change this line

https://github.com/coin-or/pulp/blob/master/src/pulp/pulp.py#L2277

@pchtsp
Copy link
Collaborator Author

pchtsp commented Mar 28, 2018

Nice, thanks. Now that I see that tests are failing I can go to bed at ease. I'll take a look tomorrow, I hope.

@pchtsp
Copy link
Collaborator Author

pchtsp commented Mar 29, 2018

quick summary of the test resuls and some analysis on them:

  • pulpTest009: returns optimal instead of infeasible
  • pulpTest011: gives non-optimal while thinking it's optimal.
  • pulpTest012: saying optimal instead of unbounded.
  • pulpTest014: get an error: unsupported operand type(s) for -: 'NoneType' and 'int'
  • pulpTest030: no relaxation, obviously.
  • pulpTest123: choco crashes instead of saying it's unbounded.

I guess I'll send the obviusly incorrect (009, 011) to the choco team via the mps so they can check them out.
For unbounded (012, 123) I think there's no support for it, although I can always ask.
In (030) we just need to put it into the conditional.
For (014), no idea.

@pchtsp
Copy link
Collaborator Author

pchtsp commented Mar 30, 2018

Current status:

remaining:

  • pulpTest009: returns optimal instead of infeasible
  • pulpTest011: gives non-optimal while thinking it's optimal.

"solved":

  • pulpTest014: I force to write an lp in order to get the duplicate variable error as the other solvers.
  • pulpTest012: saying optimal instead of unbounded => we skip the test since choco bounds all variables.
  • pulpTest030: no relaxation, obviously.
  • pulpTest123: choco crashes instead of saying it's unbounded => we skip the test since choco bounds all variables.

Now maximize gets passed correctly to choco. Some tests that are not compatible with choco were taken out.
@pchtsp
Copy link
Collaborator Author

pchtsp commented Apr 3, 2018

Hey Stu,
Could you please take a look at the PR now? I think it is now in a position to be working correctly.

thanks!

Franco

@stumitchell
Copy link
Contributor

Hi I think I will need you to to follow this process re contribution agreements https://www.coin-or.org/contributing/code/

@pchtsp
Copy link
Collaborator Author

pchtsp commented Oct 25, 2018

Hello Stuart, you mean signing the CSRO? if so, should I send it to someone in particular? you by mail for example?

@stumitchell
Copy link
Contributor

stumitchell commented Oct 25, 2018 via email

@pchtsp
Copy link
Collaborator Author

pchtsp commented Nov 3, 2018 via email

@pchtsp
Copy link
Collaborator Author

pchtsp commented Feb 4, 2019

Hey Stu, at the end are you thinking on merging this PR?

I'm thinking on adding mip start support for CPLEX_CMD and CBC but I don't really want to be having a whole bunch of different independent branches in my fork just so I can have independent PRs.

What do you think?

Franco

@stumitchell
Copy link
Contributor

Sorry to get to this so late, I'll merge if you can fix the conflicts

@pchtsp
Copy link
Collaborator Author

pchtsp commented May 14, 2019

Ok, I'll check this today / tomorrow. I've added the solver to the repository, in a similar fashion as CBC. Is it okay?

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.

2 participants