Support for mixed-integer/boolean problems (ECOS 1.1.0)

[adomahidi]

We have pushed ECOS 1.1.0 yesterday, with the major change in the repository structure - the core ecos solver is now a submodule in all interfaces:

github.com/embotech/ecos (core C API)

Interface repositories:

github.com/embotech/ecos-matlab
github.com/embotech/ecos-python

If there is any particular info you need from us to include support for binaries/integers please contact us (specifically, @hanwang who has developed the Branch-and-Bound module).

[mlubin]

Thanks for the update, it shouldn't be difficult at all to enable support for binary and integer variables.
It's not typical for Julia packages to have git submodules, and it's not really necessary here since we distribute our own binaries on OS X and Windows.

[adomahidi]

Sure, whatever works best for you! Thanks!

[tkelman]

Note that the Win64 integer size fixes necessary to compile ecos properly were reverted. ecos needs to make up its mind and be consistent about integer sizes - pick one thing and do it everywhere including throughout the testing code, whether that's platform-dependent long or 64 bit integers everywhere. Or we'll have to build patched sources.

[adomahidi]

I think I have corrected the integer issue in Win64 in the current version (1.1.1). Can you please check and re-package that one? I see that the current ECOS.jl still has version number 1.0.5 if I'm not mistaken. Thanks!! Oh and if there are more issues with the ints please let me know.

[mlubin]

Thanks @adomahidi.
Regarding the API changes for supporting integer variables, this is up for grabs by whoever needs it first :)
CC @jfsantos @karanveerm @madeleineudell

[tkelman]

How many iterations is the lp_agg test expected to take? For 32-bit binaries built from Cygwin using CC='i686-w64-mingw32-gcc -g -Wl,--stack,8388608' AR=i686-w64-mingw32-ar make -e test shared I get this output from ecostester https://gist.github.com/909202d3864727746c9f

64 bit binaries built as CC='x86_64-w64-mingw32-gcc -g -Wl,--stack,8388608' AR=x86_64-w64-mingw32-ar make -e test shared result in a segfault https://gist.github.com/bbd13b7109cc7169f7fb

[tkelman]

ecos 2.0.1 will probably not segfault any more, but I disagree with the fix made in embotech/ecos@8a26e94 - making your integer sizes depend on what compiler you're using is not a good idea, kind of the opposite of making up your mind. Pretty much means the C library's own tests aren't being run against visual studio, but that's not really our problem until a user wants to try an MSVC build of ecos along with the Julia bindings.

[adomahidi]

I'll try to explain the issue that we have in ECOS with integers, and maybe we can find a good solution together. The problem is that depending on which interface one is building, a sparse matrix comes in compressed column storage where the index arrays are integers that vary bitness on different platforms. So if we're compiling on Win64 for Matlab, then the arrays Matlab gives are 64bit, and we require to use "long ints". The issue is that in MSVC the long integer has 32bit, so you need to use "long long" (which is defined in the MSVC world as the _int64 type), whereas if you're using MinGW to compile then "long" has 64 bit size. This is at least what I have found out so far, and this is why ECOS makes the integer sizes depending on the build environment.

The cleanest solution would be to go for a predefined integer size (say int32), but then for example under Matlab Win64 we'd need to memcopy the data (a cast is not enough since sparseLDL and AMD operations perform pointer arithmetics). This would mean a performance loss, which in my view does not pay off the "cleanliness" of the solution of "making up our mind".

Of course, if you have a better proposal of how to deal with this in a better way, I'm all ears.

[tkelman]

whereas if you're using MinGW to compile then "long" has 64 bit size

This is incorrect.

[tkelman]

So the solution I think you should go for is to make the integer size configurable, with default choices that make sense for the target environment you're building for - whether that be Matlab, Python, or the standalone C library. What's missing is that in several of these configurations, most noticeably the standalone C library as we've been trying to build and report multiple times, certain integer sizes segfault because the code in ECOS is not consistent across the library and all tests. I get the distinct impression that few if any people build the library in "Matlab configuration," or "Python configuration," or even just the library by itself with MSVC, and run the C library's own tests. The environment that you're building the library for is not the same thing as the compiler that you're using to build the library. They're related in some cases, but not all.

[adomahidi]

The integer size IS configurable in the header file external/Suitesparse_config/Suitesparse_config.h. I just went through all tests and found 1 file that was using long instead of ECOS' integer type idxint. I will correct this; then all tests will be consistent with the right integer size.

As for testing, well, we run Python tests for the Python module, C tests for the C module, and Matlab tests for the Matlab module. Of course one could additionally run tests for the C core module every time any of the interfaces is built.

[adomahidi]

The test "norm.c" has been corrected in embotech/ecos@73f970c

[tkelman]

Patching a header file isn't exactly the best way of doing configuration, and you should still revert the compiler-dependent integer sizes, but yes it looks like 64-bit-ints with mingw will work now. The backtrace from February said exactly where the problem was, did it not?

[adomahidi]

Which post do you mean exactly? Apologies if this was slow, but if you know exactly where the problem is, why not create a PR next time?

[tkelman]

#16 (comment) - I was under the assumption you knew your source code better than I do. That pointed to the file in which the segfault was happening, and while I could've guessed it was an integer size problem, I didn't read the test files all that closely. I've also had a trivial 2-line PR open since March that hasn't been merged, so...

[adomahidi]

Yeah sorry for that one. I should have merged it right away.

[tkelman]

Anyway, now that that blocker is resolved, we can update the Windows and OSX binaries and the Julia bindings to use the latest version of ECOS pretty much at any time someone wants to work on that update. We can replace any instance of Clong in the Julia bindings with Int if we build the win64 binary using 64-bit integers. Should not be much work.

[adomahidi]

OK, thanks. Sorry again for not being responsive to this - try to do it better next time!

[matbesancon]

@mlubin should this issue be closed too then?

[mlubin]

Not sure. ecos_bb is still a missing feature, but not something I'd prioritize given my comment in #38 (comment).

[odow]

Closing because I don't see us adding this, and there has been no progress in 8 years.

If future reader has a need for this and it isn't met by using Pajarito.jl instead, please comment and I'll re-open.