jump-dev / ECOS.jl

A Julia interface to the ECOS conic optimization solver
https://github.com/embotech/ecos
Other
41 stars 17 forks source link

fix #7 #8

Closed tkelman closed 10 years ago

tkelman commented 10 years ago

fixes #7

32 bit https://ci.appveyor.com/project/tkelman/ecos-jl/build/1.0.3/job/owy7fobin6qaslc3 64 bit https://ci.appveyor.com/project/tkelman/ecos-jl/build/1.0.3/job/vnfxkgon65el7mn9

I see quite a bit more test output on a Linux machine than I do on Win32, but the tests apparently aren't flagging any errors? And Win64 segfaults. Smells like an integer size problem, either in the Julia bindings or in ECOS itself.

IainNZ commented 10 years ago

@tkelman to the rescue :D

The verbosity of the solver is a compile-time thing so that might explain the reduced test output - the tests are pretty decent so if they pass, it works.

I was expecting you, if anything, to say that Win32 segfaults, because its only been used/tested on 64-bit linux and OSX so far!

IainNZ commented 10 years ago

It really is great that Win32 works though. I wonder how to go about debugging the Win64 issues... Maybe going over the structs in src/types.jl with a fine-tooth comb...

IainNZ commented 10 years ago

One Win64 positive: the call to ECOS_ver at the start of test/direct.jl worked!

tkelman commented 10 years ago

Kind of a drive-by, I saw Michael Grant posting on julia-users and was wondering what was up with CVX since the Stanford guys never followed up on my standing offer to do this.

I think the problem is more likely in upstream ECOS (see tons of warnings here https://gist.github.com/ced0bf8e2f0716472438, and the 64-bit ecostester.exe does not work, at least from MinGW GCC), with differences in the size of long, etc. AIUI ECOS considers using ANSI C instead of C99 a feature, so using actual portable fixed-size integers is harder. And they have Python bindings to worry about, the situation there with respect to integer size on Win64 is a complete and utter mess (http://mail.scipy.org/pipermail/numpy-discussion/2014-July/070726.html).

IainNZ commented 10 years ago

Yes we also saw that, haven't heard from CVX.jl people lately and not really sure what the development plan is. Assume he knows about what they've done and isn't doing an independent port. In the meantime, always good to have more solvers + have the conic interface ready for a CVX-style package to use.

echu commented 10 years ago

Hey, sorry to hijack this thread a little bit: I saw mention of Michael Grant and CVX. I can probably surface some of your concerns / issues to him if you let me know what they are. As in, what do you @tkelman mean, "What's up with CVX?"

tkelman commented 10 years ago

Maybe Iain's thoughts are different than mine, but in my case it was more like a "oh cool good to see him using Julia and asking questions." And I met Madeleine, Karanveer, and the rest of the CVX.jl group a few months ago at a Bay Area Julia Users meetup, and offered to help out with Windows binaries. But we haven't seen much activity since the end of the semester, which is maybe not so surprising.

echu commented 10 years ago

Oh, okay. :-p

Well, Madeleine is sharing an office with me (she's in SF today), but I'll be sure to tell her that you guys want to know what's up with CVX.jl. :)

On Thu, Jul 31, 2014 at 10:32 AM, Tony Kelman notifications@github.com wrote:

Maybe Iain's thoughts are different than mine, but in my case it was more like a "oh cool good to see him using Julia and asking questions." And I met Madeleine, Karanveer, and the rest of the CVX.jl group a few months ago at a Bay Area Julia Users meetup, and offered to help out with Windows binaries. But we haven't seen much activity since the end of the semester, which is maybe not so surprising.

— Reply to this email directly or view it on GitHub https://github.com/JuliaOpt/ECOS.jl/pull/8#issuecomment-50791561.

IainNZ commented 10 years ago

@echu we (MIT JuliaOpt people) have met the CVX.jl team and we've been working with them to help them connect to the existing solvers. Re: Michael Grant and CVX, apart from sharing @tkelman s general happiness that he is trying Julia, just wanted to make sure he wasn't duplicating work on CVX.jl

madeleineudell commented 10 years ago

@tkelman: @karanveerm has been handling the windows integration issues; i'm not sure of the status, but it would be great to get your help!

tkelman commented 10 years ago

I think the Julia wrappers are okay at this point. The C tests of ECOS itself have some trouble too on Win64, we're tracking that at https://github.com/ifa-ethz/ecos/issues/68. Although it makes me wonder whether it has worked correctly with Windows 64-bit via Python or Matlab yet.

IainNZ commented 10 years ago

We could just merge this for now and get it out there for everyone not using Julia Win64 I suppose...

jfsantos commented 10 years ago

Since apparently the wrappers are ok and this is a problem in ECOS, I agree with @IainNZ that we could merge this. As soon as they get it fixed, hopefully we will just need to update the version string in our build script.

tkelman commented 10 years ago

Okay by me. Someone (probably me? could also be cross-compiled and should come out the same) will have to build and upload new Windows binaries once the issue is fixed upstream, then we'll know for sure whether the Julia code here needs any changes for Win64.

IainNZ commented 10 years ago

OK, pulling trigger.

tkelman commented 10 years ago

As I suspected it's an integer size problem. SuiteSparse_config.h uses C99 64-bit integers for indices on Win64, but ECOS has a lot of places (esp. in the tests) where it incorrectly uses long. Depending which way they decide to go, we may need to replace Clong with Int here in the Julia wrappers.