Memory leak for IntegralLattice

[Simon Brandhorst]

Computing the discriminant group of an integral lattice causes both to persist in memory.

sage: L = IntegralLattice("A2") 
sage: for k in range(1,500): 
....:     G = L.twist(k) 
sage: gc.collect() 
sage: len([a for a in gc.get_objects() if type(a)==type(L)])                                                                                                                                       
1058
3
sage: L = IntegralLattice("A2") 
sage: for k in range(1,500): 
....:     G = L.twist(k) 
....:     D = G.discriminant_group() 
sage: gc.collect() 
sage: len([a for a in gc.get_objects() if type(a)==type(L)])                                                                                                                                       
1284
501

Creating thousands of lattices and computing their discriminant groups is actually a common use case. Serious computations are impossible with this kind of memory leak.

[Simon Brandhorst]

L.discriminant_group() is cached. Removing the cache is a possible fix to the leak. However it seems very natural for a lattice to remember its discriminant group. And is basically the only reasonable way to create such things. On the other hand the discriminant group knows about its lattice covering lattice (it has to).

Another option to fix the leak is to remove CachedRepresentation from the discriminant group.

Not sure how to proceed here. And there are other instances of this kind of memory leak.

[Travis Scrimshaw]

I would probably remove the @cached_method from discriminant_group(). I don't see immediately why there would be a need to cache that. It doesn't seem like something you call frequently from different parts of the code. You could just tweak maximal_overlattice() in order to avoid repeated calls (which I would argue is better coding style anyways). (Similarly, you don't cache the orthogonal_group().)

[Simon Brandhorst]

discriminant_group() is something that is called quite frequently by the user. Creating it is not too expensive so okay.

But computing generators for the orthogonal_group() is very expensive. So I think here unique representation does not help a lot. Storing it on its domain seems better.

[Simon Brandhorst]

I guess I can let the unique representation machinery do the caching and only store the generators on the orthogonal group. So that is fine. Final question how best to doctest that a memory leak is fixed?

[Travis Scrimshaw]

You could use the test in the ticket description, but it doesn't necessarily need a doctest IMO.

[Simon Brandhorst]

New commits:

​3ee728a

remove @cached_method to fix a memory leak for IntegralLattice

[Travis Scrimshaw]

Your """ is not aligned properly (see the diff) and your added doctests need to be indented.

(I am slightly squeamish about adding such a doctest for the memory leak, but not enough to not include it.)

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​bd6c256

indentation

[Travis Scrimshaw]

Thank you. LGTM.