Opened 9 years ago

Closed 9 years ago

#10151 closed defect (fixed)

Update calls to MixedIntegerLinearProgram and its solve function to follow the new interface

Reported by: ncohen Owned by: ncohen
Priority: major Milestone: sage-4.6.1
Component: linear programming Keywords:
Cc: malb, mvngu Merged in: sage-4.6.1.alpha1
Authors: Nathann Cohen Reviewers: Martin Albrecht
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by ncohen)

Right now, the followinjg is happening

sage: g = graphs.PetersenGraph()
sage: g.dominating_set()
IBM ILOG License Manager: "IBM ILOG Optimization Suite for Academic Initiative" is accessing CPLEX 12 with option(s): "e m b q ".
[4, 6, 7]
sage: g.dominating_set(solver = "GLPK")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)

/auto/sop-nas2a/u/sop-nas2a/vol/home_mascotte/ncohen/<ipython console> in <module>()

/home/ncohen/sage/local/lib/python2.6/site-packages/sage/graphs/generic_graph.pyc in dominating_set(self, independent, value_only, solver, verbose)
   5666             return p.solve(objective_only=True, solver=solver, log=verbose)
   5667         else:
-> 5668             p.solve(solver=solver, log=verbose)
   5669             b=p.get_values(b)
   5670             return [v for v in g.vertices() if b[v]==1]

/home/ncohen/sage/local/lib/python2.6/site-packages/sage/numerical/mip.so in sage.numerical.mip.MixedIntegerLinearProgram.solve (sage/numerical/mip.c:5813)()

ValueError: Solver argument deprecated. This parameter now has to be set when calling the class' constructor

The new interface between Sage and the LP solvers requires the solver's name to be given when the constructor is called.

This patch applied, all is fine :

sage: g = graphs.PetersenGraph()
sage: g.dominating_set()
IBM ILOG License Manager: "IBM ILOG Optimization Suite for Academic Initiative" is accessing CPLEX 12 with option(s): "e m b q ".
[4, 6, 7]
sage: g.dominating_set(solver = "GLPK")
[0, 2, 6]

Apply first :

This long list of undeserved dependencies is here to prevent this patch from having to be rebased, as it touches many files almost everywhere.

Attachments (1)

trac_10151.patch (44.4 KB) - added by ncohen 9 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by ncohen

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by mvngu

  • Cc mvngu added

comment:3 Changed 9 years ago by malb

  • Status changed from needs_review to needs_work

I think it would be good if the user could set some global option which solver to use, such that (s)he doesn't have to give the solver each time a high-level function is called.

comment:4 Changed 9 years ago by ncohen

Well, the "default solver" is already automatically picked as the best among those installed. Cplex is taken if available, then Coin, then GLPK (default, always there).. This is mainly for checking that the results are coherent when the solver changes...

How do you think such a global option should be set/read ?.. I do not know the custom for this in Sage ^^;

Nathann

comment:5 follow-up: Changed 9 years ago by malb

Sure, you pick the overall best, but for a particular problem another one might be better and it would be nice to force a particular solver. Another application would be benchmarketing.

I'd suggest this user interface:

sage: default_mip_solver()
'Coin'
sage: default_mip_solver('GLPK')
sage: default_mip_solver()
'GLPK'

comment:6 in reply to: ↑ 5 Changed 9 years ago by ncohen

Sure, you pick the overall best, but for a particular problem another one might be better and it would be nice to force a particular solver.

Well, then for a *particular* problem the solver parameter should be the perfect answer, wouldn't it ? :-p

I'll be writing this patch, it shouldn't take long :-)

Nathann

comment:7 Changed 9 years ago by malb

Maybe you're right and we don't need it, I don't know :) Your choice then.

comment:8 Changed 9 years ago by ncohen

Hmmm... I almost finished it, but now I am wasting time on a stupid thig... I defined in sage.numerical.backends.generic_backend, at the module level, a variable default_solver which caches the default solver to use.... Of course, when the default_mip_solver method changes its values, it only changes the value of a default_solver variable defined INSIDE of the default_mip_variable method, which has absolutely no effect on the "global" variable...

Do you know which trick to use in this case ? ^^;

Nathann

comment:9 Changed 9 years ago by malb

Hi, do you mean:

variable = False

def foo():
   global variable
   variable = True

foo()
print variable

comment:10 Changed 9 years ago by ncohen

  • Status changed from needs_work to needs_review

exactly... thank you :-)

Nathann

comment:11 Changed 9 years ago by malb

Hi, I get the follow doctest failures:

malb@geom:~/scratch/sage-4.6.1.alpha0$ ./sage -t -long  devel/sage/sage/graphs/graph_coloring.py
sage -t -long "devel/sage/sage/graphs/graph_coloring.py"
**********************************************************************
File "/scratch/malb/sage-4.6.1.alpha0/devel/sage/sage/graphs/graph_coloring.py", line 1111:
    sage: g1,g2 = linear_arboricity(g, k=0)
Exception raised:
    Traceback (most recent call last): 
    ...
        p = MixedIntegerLinearProgram(solver = solver)
    NameError: global name 'solver' is not defined

comment:12 Changed 9 years ago by malb

I think

:meth:`default_mip_solver` -- Returns/Sets the default MIP solver. 

should be :func:.

Other than that, the patch looks good. So once all doctests pass and this is fixed, I'm happy to give this patch a positive review.

comment:13 Changed 9 years ago by ncohen

Sorry about that... :-/

I was pretty sure I had checked it passed all tests... :-/

Updated.

Nathann

Changed 9 years ago by ncohen

comment:14 Changed 9 years ago by malb

  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to positive_review

comment:15 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.