Fix solver option handling: names was deprecated in favor of fieldnames

[czlee]

This error was happening:

ERROR: LoadError: type DataType has no field names
 in model at /home/czlee/.julia/v0.4/GLPKMathProgInterface/src/GLPKInterfaceMIP.jl:187
 in buildInternalModel at /home/czlee/.julia/v0.4/JuMP/src/solvers.jl:236
 in solve at /home/czlee/.julia/v0.4/JuMP/src/solvers.jl:77
 [inlined code] from util.jl:155
...

I think the issue occurred because typeof(lpm.param).names stopped making sense when they deprecated names in favor of fieldnames at JuliaLang/julia#10332, and changed DataType.names to DataType.name.names in JuliaLang/julia@afd4eb0, both of which are introduced in Julia 0.4.

fieldnames isn't defined in Julia 0.3 though (as far as I can tell), so I presume this would break 0.3 compatibility.

I'm new to Julia, so apologies if I got something wrong.

[mlubin]

Thanks! Are there any tests you could add to test/runtests.jl which would catch this issue? It seems to have gone unnoticed for quite a while.

[czlee]

I'm sure that shouldn't be too hard, it was a pretty reliable bug. It should be reproducible when passing pretty much any option to GLPKSolverMIP and then attempting to run the solver. I'm using JuMP, which might complicate things a little with my (lack of) understanding of how GLPKSolverMIP is used, but will have a think about short snippets to test for it that don't rely on other packages. 😃

[mlubin]

here you could add a call to mixintprogtest with some options

[czlee]

Since I know the bug occurs in the model function, the easiest and narrowest (and least redundant) test would be just to add something like:

GLPKInterfaceLP.model(GLPKSolverLP(it_lim=-1))
GLPKInterfaceMIP.model(GLPKSolverMIP(it_lim=-1))

to runtests.jl. (Either it runs without error or it doesn't.)

Alternatively as you suggest I could add:

linprogsolvertest(GLPKSolverLP(it_lim=-1))
mixintprogtest(GLPKSolverMIP(it_lim=-1))

It's worth noting here that the it_lim=-1 should (in theory) do absolutely nothing to the solver: -1 is the default for the ItLim parameter. I worry that putting in more interesting options, e.g. it_lim=1000 or tol_obj=1e-2, would throw off the tests that check the actual solutions. I mean, those options don't cause tests to fail now (I tried), but given that the tests are in a different package, you presumably don't want a test whose purpose is to check option handling, to cause a test to fail because the return solution wasn't the true optimum. (Say, if MathProgInterface changes its tests in future, and the new tests wouldn't be valid tests when run with our options here.)

Do you have a view on which is preferable?

[mlubin]

Yes, I would avoid setting options that could result in early termination. Something like msg_lev seems harmless though.

[carlobaldassi]

Compatibility with julia 0.3 should be ensured by Compat, which defines fieldnames.
My preference for testing is to do it directly (first option).

[mlubin]

@carlobaldassi, for the first option, it would be nice to test that the options are actually set and not ignored. What's a good way to do this?

[czlee]

I added the first option, and also some tests to check that the parameters were in fact set, by checking the param and smplxparam members of the returned models. Because the models aren't used to try to solve the test problems, I just picked arbitrary values that are unlikely to be defaults.

[mlubin]

Thanks!