search

rust-or / rust-lp-modeler

Lp modeler written in Rust
MIT License
96 stars 29 forks source link

Feature: native coin-or solver #49

Closed carrascomj closed 4 years ago

carrascomj commented 4 years ago

Addresses #45

Description

This PR provides a new solver to call CoinOR CBC trough the C API.

Implementation

The new struct NativeCBCSolver implements SolverTrait and uses the rust bindings in https://github.com/KardinalAI/coin_cbc/. A different NativeSolverTrait should be considered to extend the functionality in the future.

carrascomj commented 4 years ago

It compiles just fine on linux 5.9.1 but that does not seem to be the case on Windows. It needs more testing.

jcavat commented 4 years ago

Thank you for your contribution. Can you find out where the problem is coming from? Compatibility with OS is quite important I think

carrascomj commented 4 years ago

It seems like the cbc_coin bindings are looking for CbcSolver.lib, since that's the name of the library in Linux systems. However, looking at the file that you use in appveyor, it seems like the library is called libCbcSolver.lib for Windows.

If I am correct, it would be a matter of conditional compilation. I'll open an issue at https://github.com/KardinalAI/coin_cbc/.

I don't have access right now to a Windows system (I will have it next week maybe).

EdorianDark commented 4 years ago

The difference between this and the other backends is, that this has compile time dependencies. So making it an optimal feature would be a good idea. The feature could be only available on Linux, which would solve this problem.

carrascomj commented 4 years ago

@EdorianDark Maybe we can do that if there's no way to make it compile in Windows (which I think it's just me lacking the knowledge to do it). However, it is not a solution but a workaround, and I am sure that some people would like to get it working on Windows. Like I said, I could get my hands on a Windows computer this coming week to see if I can make it work.

jcavat commented 4 years ago

Ok, thanks. Let me know if we can fix this issue. In last resort, we can imagine providing your implementation as an extension of the core lib. But it can be better to merge it here.

jcavat commented 4 years ago

I've merge this PR and #51 but wait for a fix before pushing into production for the next version

jcavat commented 4 years ago

All good. I keep only linux tests for the CI. Thanks