Ticket #12903 (closed defect: fixed)

Opened 14 months ago

Last modified 14 months ago

Memory leaks with CPLEX

Reported by: ncohen Owned by: ncohen
Priority: major Milestone: sage-5.1
Component: linear programming Keywords:
Cc: dcoudert Work issues:
Report Upstream: N/A Reviewers: David Coudert
Authors: Nathann Cohen Merged in: sage-5.1.beta0
Dependencies: Stopgaps:

Description

Some memory leaks remained in Sage's CPLEX interface, as reported by Geoff on  http://ask.sagemath.org/question/1170/memory-blowup-with-milp

Fixed there, after a lot of debugging :-P

You can check the difference by running the following piece of code :

while True:
    p = MixedIntegerLinearProgram(solver = "CPLEX")
    get_memory_usage()

The result is scary.

Nathann

Attachments

trac_12903.patch Download (6.1 KB) - added by ncohen 14 months ago.

Change History

comment:1 Changed 14 months ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 14 months ago by dcoudert

  • Status changed from needs_review to needs_work

Hello,

the patch is working (install OK, long tests OK, docbuild OK). However, we still have memory leaks when using try except.

This example without try/except uses constant memory, but with try/except, memory is constantly increasing.

def test(G, solver = None, verbose = 0):
    from sage.numerical.mip import MixedIntegerLinearProgram, MIPSolverException, Sum 
    p = MixedIntegerLinearProgram(solver = solver, maximization = False)
    b = p.new_variable(dim=1)
    for u,v in G.edge_iterator(labels=False):
        p.add_constraint(b[u]+b[v] >= 1)
    p.set_objective(Sum([b[u] for u in G]))
    p.set_binary(b)
    try:
        p.solve()
        sol = p.get_values(b)
        return len([u for u in G if sol[u]==1])
    except MIPSolverException: 
        print "oups"
        return 0

def my_test(n,p,sample=1000):
    for i in xrange(sample):
        g = graphs.RandomGNP(n,p)
        res = test(g, solver = "CPLEX", verbose = 0)
        print res, get_memory_usage()
sage: my_test(100,0.1, sample = 10)
69 388.94140625
68 388.94140625
69 389.0703125
71 389.0703125
68 389.0703125
71 389.0703125
69 389.19921875
69 389.19921875
71 389.19921875
71 389.19921875

comment:3 Changed 14 months ago by ncohen

  • Status changed from needs_work to needs_review

Hellooooooo !!

Well, it actually isn't related to try/catch (your LP computes a vertex cover --> it never fails) but there is a leak indeed :-D

I reduced your code to that and the problem remains

for i in xrange(5000):
    p = MixedIntegerLinearProgram(solver = "CPLEX", maximization = False)
    b = p.new_variable(dim=1)
    for u,v in [(0, 1), (0, 4), (0, 5), (1, 2), (1, 6), (2, 3), (2, 7), (3, 4), (3, 8), (4, 9), (5, 7), (5, 8), (6, 8), (6, 9), (7, 9)]:                                                                                                                                                                                                                                   
        p.add_constraint(b[u]+b[v] >= 1)
    print get_memory_usage()

That is fixed by some additional sage_free calls.... All the method had them except two, add_linear_constraint included. -_-

By the way, I am a bit scared... Do the tests pass for you when having CPLEX installed as the default solver ? I see a broken docstring in generic_graph in the traveling_salesman_problem method. The bug is not realted to this ticket (it appears regardless of whether the current patch is applied) and I have no idea where it comes from O_o

Nathann

Changed 14 months ago by ncohen

comment:4 Changed 14 months ago by dcoudert

Which version are you using? I have no docstring error with 5.0.rc0.

D.

comment:5 Changed 14 months ago by ncohen

I'm using rc0 too O_o

So for you this ticket passes all tests ? Well, as I said earlier the doctests breaks on my machine regardless of whether it is applied anyway... O_o

Nathann

comment:6 Changed 14 months ago by dcoudert

  • Status changed from needs_review to positive_review
  • Reviewers set to David Coudert
  • Authors set to Nathann Cohen

That's much better!!!

From my side the patch is ready to go (long tests, docbuild, stable memory,...) and more than useful ;)

comment:7 Changed 14 months ago by ncohen

Thaaaaaaaanks ! :-)

Nathann

comment:8 Changed 14 months ago by jdemeyer

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