ryanjoneil / SCIP.jl

(deprecated) Julia interface to the SCIP solver
MIT License
2 stars 4 forks source link

[WIP] Refactor codebase, MPB interface #6

Closed joehuchette closed 10 years ago

joehuchette commented 10 years ago

Not ready to merge yet, this is mostly to get a repo-level diff at this point. I'm going to keep playing around with how best to represent Ptr{Void} in SCIP. Also it'd probably be best to update the XML parsers to match the changes I made in the wrapper before merging.

ryanjoneil commented 10 years ago

OK, let me know when you're ready. I'll work on the XML parser and try to get that matching your changes in the meantime.

joehuchette commented 10 years ago

So I'm thinking now the nicest interface will be to keep the *_t types (but without leading underscore, since they're not part of the C API per se), and have pointer return a pointer to that object that works nicely with the C wrappers. The pointer types should also have clean constructors, so you can write, e.g.

function SCIPcreate()
    scip = SCIP_t()
    _SCIPcreate(pointer(scip))
    finalizer(scip, scip->_SCIPfree(scip))
    return scip
end
mlubin commented 10 years ago

That sounds right.

joehuchette commented 10 years ago

Alright @ryanjoneil, I think I've settled on an approach I'm happy with. Let me know what you think; if you agree, we can go ahead and update the xml parser accordingly.

1) Add a leading underscore to everything wrapped from the C API. That is, SCIPcreate becomes _SCIPcreate, SCIP_Bool becomes _SCIP_Bool, etc. 1a) The helper pointer types (e.g. SCIP_t) do not have leading underscores (since they are not part of the interface strictly, I'm also hoping to move this code into a separate submodule. More below.) 2) Add helper default constructors for the pointer helper types. They can be one-liners that look like

SCIP_t() = SCIP_t(Array(Ptr{_SCIP},1))

3) Remove the array methods. 4) The pointer methods should now be

Base.pointer(scip::_SCIP_t) = pointer(scip.array_ptr_scip)

5) Add convert methods like

Base.convert(::Type{Ptr{_SCIP}},scip::_SCIP_t) = scip.array_ptr_scip[1]

6) In scip_functions.jl, remove the type annotations in the method signature for anything with a pointer in it (or a pointer to a SCIP object, things like *Char should be fine). This allows us to pass in SCIP_t in place of Ptr{_SCIP}, and the convert method from (5) will be invoked automatically in the ccall. 7) Add an export ... line at the top of all wrapper files that explicitly exports all the _SCIP* wrappings. This helps with having the wrappers live in a submodule.

Hopefully this makes sense; I may have forgotten something. Let me know what you think.

joehuchette commented 10 years ago

Oh, and the function defs in scip_functions.jl shouldn't explicitly call pointer or array anymore, e.g.

_SCIPcreateVarBasic(scip, var, name::String, lb::_SCIP_Real, ub::_SCIP_Real, obj::_SCIP_Real, vartype::_SCIP_VARTYPE) = @scip_ccall_check("SCIPcreateVarBasic", (Ptr{_SCIP}, Ptr{Ptr{_SCIP_VAR}}, String, _SCIP_Real, _SCIP_Real, _SCIP_Real, _SCIP_VARTYPE), scip, var, name, lb, ub, obj, vartype)

You would call this with something like

_SCIPcreateVarBasic(scip, pointer(var), "myvar", 1.0, 2.0, 3.0, SCIP._SCIP_VARTYPE_INTEGER)
joehuchette commented 10 years ago

Actually, I have an idea to replace SCIP_*_t with SCIPPtr{_SCIP} that means we won't have to deal with generating all that code. Let me play around with it.

joehuchette commented 10 years ago

Alright so I managed to excise SCIP_*_t completely, so that doesn't need to be in the xml parser. I changed all the SCIP types from typealiases for Void to their own (empty) types:

abstract SCIPType
type _SCIP <: SCIPType end
ryanjoneil commented 10 years ago

Sorry for the delay. I am back now and should have some time to make the changes.

Everything sounds reasonable. I'm working on 1 and 2. Should have changes for those checked in to the xml-parser branch tomorrow or Thursday.

joehuchette commented 10 years ago

Cool, sounds good. We should merge both branches (together) at some point soon, we're making some good progress!

joehuchette commented 10 years ago

Hey @ryanjoneil, what do you think about merging this branch or your parsing branch soon? It'll be easier to work between the two when one is living on master. I'm reasonably happy with this one thus far (minimal MPB interface working), but it'll have to integrate more tightly with the parser before it's adequately featured.

ryanjoneil commented 10 years ago

My apologies, I got very waylaid by other things so this is taking longer than expected.

I've added all the underscores to the SCIP constants and functions and gotten test.jl to run again. The xml-parser branch is now merged back into master, and I'll pick up making the XML parsed output mirror the refactor branch again tomorrow. I think the rest will be relatively easy.

ryanjoneil commented 10 years ago

Hey @joehuchette -- I merged the xml-parser branch into master and am about halfway through merging the refactor branch into master as well. Should be finished tomorrow or Saturday. Will ping you when it's done.

joehuchette commented 10 years ago

Great, I won't get in your way if you want to have a go at it. Let me know if you run into any issues!

joehuchette commented 10 years ago

Cool! What should we target next?

ryanjoneil commented 10 years ago

@joehuchette: Not sure why this request got closed. The master branch is awfully close, but not quite working yet. If you have a minute, would you mind running test.jl and figuring out what is still being generated wrong in the src/ dir?

joehuchette commented 10 years ago

Hmm weird, looks like you only merged a few commits from this branch, somehow. I'll play around with it.

joehuchette commented 10 years ago

Looks like the issue is with incompatibilities with the signatures in scip_functions.jl. I'll play around with the wrapping code and see if I can't figure it out.

joehuchette commented 10 years ago

Hey @ryanjoneil I made some changes to the python templating code but I can't figure out how to run it properly. Can you give me the invocation you were using, and which directory you were running it in?

ryanjoneil commented 10 years ago

Not sure why the merge didn't happen fully, though I'm not a git expert by a long shot.

Regardless, to run the interface generator you call the following command from the SCIP.jl directory:

python bin/generate-julia-interface.py xml/ templates/ src/

You'll need to have jinja2 and lxml installed.