Opened 3 years ago

Closed 3 years ago

#12903 closed defect (fixed)

Memory leaks with CPLEX

Reported by: ncohen Owned by: ncohen
Priority: major Milestone: sage-5.1
Component: linear programming Keywords:
Cc: dcoudert Merged in: sage-5.1.beta0
Authors: Nathann Cohen Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: Commit:
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 (1)

trac_12903.patch (6.1 KB) - added by ncohen 3 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 3 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 3 years 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 3 years 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 3 years ago by ncohen

comment:4 Changed 3 years ago by dcoudert

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

D.

comment:5 Changed 3 years 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 3 years ago by dcoudert

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

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 3 years ago by ncohen

Thaaaaaaaanks ! :-)

Nathann

comment:8 Changed 3 years ago by jdemeyer

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