Feature/coin or cbc interface

[braceal]

Here is the PR promised in #212

Includes: optlang expression parsing bug fix, coinor_cbc interface, coinor_cbc test suite, and unittest interface update (changed assertEquals to assertEqual to remove depreciation warning).

Special note: I fixed an optlang bug in expression_parsing.py and wrote a short test case exposing it. It has to do with a linear objective with only a constant term. The expression was always being parsed as 0 instead of the constant term. The test case is test_constant_objective in the coinor_cbc test suite (this can be moved to a more appropriate location).

Currently passing 93/103 tests. The failing tests are:

Name change tests
ConstraintTestCase::test_change_constraint_name
ModelTestCase::test_change_constraint_name
VariableTestCase::test_change_name
test_changing_variable_names_is_reflected_in_the_solver

Changing variable/constraint names is not natively supported in MIP (python CBC interface)

Dual tests
test_integer_batch_duals
test_integer_constraint_dual
test_integer_variable_dual

Just need to know the rule on when to throw an exception

Other
test_indicator_constraint_support
test_timeout - MIP timeout setting is flaky around 0 seconds. Timeout should work for longer problems though

There are a couple small features I would like to add before a merge such as presolve, etc. I am also going to add a few more test cases which solve different example problems. But I wanted to post the code for review.

Documentation also needs to be written.

[cdiener]

This looks good! I wouldn't stress about the mip dependency issue. Python 2.7 and 3.4 are deprecated Python versions anyway and will probably be removed from the CI soon. Do all tests pass now or are you skipping some? The name changing would probably have to work since other tools like cobrapy depend on that. You would usually manage that by an internal mapping of variable/constraint IDs to solver IDs. This way if you change the variable name you would simply update the id in the mapping but maintain the solver ID...

[cdiener]

I don't think this should have been closed either...

[braceal]

Sorry for my absence. I don't think this should be closed either, the variable/constraint name changing is the last feature to add before it can be released. I've tested it with cobrapy integration and it seems to work fine and this branch is currently being used for a Flux Balance Analysis app in Kbase. I'm currently on another project, but I can try to have this finished by the end of the year.

If anyone else wants to finish this, please do. I think it is very close.

There are a few other updates that are on a separate branch where I tested the cobrapy, OSQP and COINOR-CBC together. Here is the separate branch: https://github.com/braceal/optlang/tree/test/coinor-cbc_osqp
Here is the compare: braceal/optlang@feature/coin-or-cbc-interface...braceal:test/coinor-cbc_osqp

[Midnighter]

Sorry, I know what happened now. I had activated auto-deletion of head branches after merge. @KristianJensen's PR actually merged devel into master. Then devel got deleted and thus all feature branches based on devel got automatically closed and deleted. My apologies. I have removed the setting and will bring back the branches/PRs.

[cdiener]

Seems to fail due to some incompatibilities with symengine. Might have to adjust tests as well since with symengine you might get expressions like 0.0 + x which often do not evaluate equal to x.

[carrascomj]

MIP does not support python2, so this may be a good chance to consider removing support for it in optlang too.

[braceal]

I just recalled, I left a TODO to handle before merge. @cdiener I think this had to do with cobrapy pickle files, do you know what the correct behavior should be here?

https://github.com/braceal/optlang/blob/feature/coin-or-cbc-interface/src/optlang/coinor_cbc_interface.py#L536

[cdiener]

That looks good it's the same as in https://github.com/braceal/optlang/blob/feature/coin-or-cbc-interface/src/optlang/osqp_interface.py#L625-L631 right? The configuration needs to pickle the tolerances as well. Cobrapy pickles will be regenerated with new optlang releases so that will work.

[braceal]

Ok great, thanks for reviewing! Yes, I agree the logic is the same as in the link you posted. The TODO can probably be removed then.

[cdiener]

I think this should behave like the other optional solvers. You can use the tox sections to only install mip for Python 3 so it will not run the cbc tests for Python 2.

[cdiener]

Fails because there are f-strings in test_coinor_cbc_interface.py and that will trigger a syntax error on Python 2 when the file is read.

[carrascomj]

Ups, I didn't see that. Working on it!

[carrascomj]

If the requested changes were solved for you @cdiener, I think this can be merged.

[cdiener]

Awesome, thanks for carrying this to the finish line.

[braceal]

Thank you both so much for seeing this through to completion! @cdiener @carrascomj 🎉