Sage: Ticket #14058: Weakly reference binary operation codomains
https://trac.sagemath.org/ticket/14058
<p>
R(1) + S(1) caches a strong reference to both R and S, which we'd like to avoid.
</p>
<p>
See discussion at <a class="ext-link" href="https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/ozhIHIwQ4WA"><span class="icon"></span>https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/ozhIHIwQ4WA</a>
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/14058
Trac 1.1.6robertwbTue, 05 Feb 2013 09:57:39 GMTattachment set
https://trac.sagemath.org/ticket/14058
https://trac.sagemath.org/ticket/14058
<ul>
<li><strong>attachment</strong>
set to <em>14058-weak-binop.patch</em>
</li>
</ul>
TicketrobertwbTue, 05 Feb 2013 10:00:37 GMTstatus changed; cc set
https://trac.sagemath.org/ticket/14058#comment:1
https://trac.sagemath.org/ticket/14058#comment:1
<ul>
<li><strong>cc</strong>
<em>nbruin</em> <em>SimonKing</em> <em>jpflori</em> added
</li>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketSimonKingTue, 05 Feb 2013 10:49:07 GMT
https://trac.sagemath.org/ticket/14058#comment:2
https://trac.sagemath.org/ticket/14058#comment:2
<p>
Is it possible to add an example that shows that parents become collectable that previously weren't?
</p>
TicketnbruinTue, 05 Feb 2013 17:38:12 GMTattachment set
https://trac.sagemath.org/ticket/14058
https://trac.sagemath.org/ticket/14058
<ul>
<li><strong>attachment</strong>
set to <em>trac_14058-doctest.patch</em>
</li>
</ul>
<p>
Doctest proposal
</p>
TicketnbruinTue, 05 Feb 2013 17:42:54 GMT
https://trac.sagemath.org/ticket/14058#comment:3
https://trac.sagemath.org/ticket/14058#comment:3
<p>
I like the idea. Can we get some data on speed regressions due to this? In principle the double lookup might be noticeable. I don't mean speed regressions due to parents getting deleted -- those are good to know but need other fixes. I mean when the parents are still around.
</p>
<p>
Attached doctests may need <a class="closed ticket" href="https://trac.sagemath.org/ticket/12313" title="defect: Fix yet another memory leak caused by caching of coercion data (closed: fixed)">#12313</a> to work, in which case this ticket should depend on that (because then that's necessary to get benefit from this ticket).
</p>
TicketnbruinTue, 05 Feb 2013 19:54:56 GMT
https://trac.sagemath.org/ticket/14058#comment:4
https://trac.sagemath.org/ticket/14058#comment:4
<p>
Sadly, the performance hit is quite noticeable.
</p>
<pre class="wiki">a=ZZ(1)
b=QQ(1)
c=ZZ['x'](1)
d=b+c
def test(x,y):
for i in xrange(10^6):
_=x+y
</pre><p>
Within a run, timing <code>test</code> seems fairly consistent. However, between different compiles of the same code I'm observing fluctuations of up to <code>0.10s</code> in timings. Lucky/unlucky memory layout?
Anyway, with patch:
</p>
<pre class="wiki">sage: %time test(a,b)
CPU times: user 2.15 s, sys: 0.00 s, total: 2.15 s
Wall time: 2.16 s
sage: %time test(a,c)
CPU times: user 2.25 s, sys: 0.00 s, total: 2.25 s
Wall time: 2.26 s
sage: %time test(b,c)
CPU times: user 16.83 s, sys: 0.00 s, total: 16.83 s
</pre><p>
without patch
</p>
<pre class="wiki">sage: %time test(a,b)
CPU times: user 1.54 s, sys: 0.00 s, total: 1.54 s
Wall time: 1.55 s
sage: %time test(a,c)
CPU times: user 1.64 s, sys: 0.00 s, total: 1.64 s
Wall time: 1.64 s
CPU times: user 15.68 s, sys: 0.00 s, total: 15.68 s
Wall time: 15.76 s
</pre><p>
For <code>test(a,a)</code>, <code>test(b,b)</code>, <code>test(c,c)</code> there doesn't seem to be a difference (luckily!)
</p>
<p>
The penalties seem to be about <code>0.6s</code>, <code>0.6s</code>, <code>1.2s</code>, which is rather consistent with 1,1,2 extra lookups.
</p>
<p>
This indicates to me it may be worth trying storing a weakref to the morphism instead, since that can probably be dereferenced faster than a coercion lookup on the codomain. Logically this should be equivalent. We're just storing a weakref to the object we're interested in rather than instructions where to go and lookup the object we want.
</p>
<p>
Caveat:
</p>
<p>
For stored coercions of the type
<code>(R,S, ...): (None,<map S->R>)</code>
or
<code>(R,S, ...): (<map R->S>,None)</code>
the codomain is part of the key, so deletion of the codomain (which is a prerequisite for the morphism being deleted and hence the weakref dying) implies removal of the weak key entry in <code>TripleDict</code>.
</p>
<p>
However, for stored coercions of the type
<code>(R,S,...): (<map R->C>,<map S->C>)</code>
we could have that <code>C</code> and the weakrefs to the morphisms die, but that would not trigger the removal from the <code>TripleDict</code>. So we could still encounter dead weakrefs (but much less than one would perhaps initially think!) It's not really much different from how <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14058/14058-weak-binop.patch" title="Attachment '14058-weak-binop.patch' in Ticket #14058">14058-weak-binop.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14058/14058-weak-binop.patch" title="Download"></a> handles dead weakrefs.
</p>
<p>
Major problem for immediately applying this idea:
</p>
<pre class="wiki">sage: M=QQ.coerce_map_from(ZZ)
sage: import weakref
sage: weakref.ref(M)
TypeError: cannot create weak reference to 'sage.rings.rational.Z_to_Q' object
</pre>
TicketrobertwbTue, 05 Feb 2013 20:02:44 GMT
https://trac.sagemath.org/ticket/14058#comment:5
https://trac.sagemath.org/ticket/14058#comment:5
<p>
Yeah, I had exactly this same idea. I'll post an updated patch.
</p>
TicketrobertwbTue, 05 Feb 2013 20:24:41 GMTattachment set
https://trac.sagemath.org/ticket/14058
https://trac.sagemath.org/ticket/14058
<ul>
<li><strong>attachment</strong>
set to <em>14058-weak-binop-morphisms.patch</em>
</li>
</ul>
TicketrobertwbTue, 05 Feb 2013 20:25:21 GMT
https://trac.sagemath.org/ticket/14058#comment:6
https://trac.sagemath.org/ticket/14058#comment:6
<p>
Apply 14058-weak-binop-morphisms.patch and trac_14058-doctest.patch
</p>
TicketnbruinTue, 05 Feb 2013 21:16:57 GMT
https://trac.sagemath.org/ticket/14058#comment:7
https://trac.sagemath.org/ticket/14058#comment:7
<p>
Great! With this new patch I cannot distinguish the timing differences from the noise one normally gets already, so I'd think this is quite acceptable.
</p>
TicketnbruinTue, 05 Feb 2013 22:12:05 GMTdependencies set
https://trac.sagemath.org/ticket/14058#comment:8
https://trac.sagemath.org/ticket/14058#comment:8
<ul>
<li><strong>dependencies</strong>
set to <em>#12313</em>
</li>
</ul>
<p>
patchbot seems unhappy about the doctest.Probably <a class="closed ticket" href="https://trac.sagemath.org/ticket/12313" title="defect: Fix yet another memory leak caused by caching of coercion data (closed: fixed)">#12313</a> is indeed necessary to make parents reclaimable.
</p>
TicketSimonKingWed, 06 Feb 2013 07:26:13 GMTdescription changed
https://trac.sagemath.org/ticket/14058#comment:9
https://trac.sagemath.org/ticket/14058#comment:9
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14058?action=diff&version=9">diff</a>)
</li>
</ul>
<p>
Stating in the ticket description what patches are to apply. Admittedly I didn't test yet whether the doctest patch applies after Robert's new patch. But the test patch is needed, I think.
</p>
<p>
Apply 14058-weak-binop-morphisms.patch and trac_14058-doctest.patch trac_14058-doctest.patch
</p>
TicketSimonKingWed, 06 Feb 2013 07:26:45 GMTdescription changed
https://trac.sagemath.org/ticket/14058#comment:10
https://trac.sagemath.org/ticket/14058#comment:10
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14058?action=diff&version=10">diff</a>)
</li>
</ul>
TicketSimonKingWed, 06 Feb 2013 13:45:57 GMT
https://trac.sagemath.org/ticket/14058#comment:11
https://trac.sagemath.org/ticket/14058#comment:11
<p>
The patches apply, and at least the new test passes. So, this together with <a class="closed ticket" href="https://trac.sagemath.org/ticket/12313" title="defect: Fix yet another memory leak caused by caching of coercion data (closed: fixed)">#12313</a>, does fix another leak.
</p>
TicketSimonKingWed, 06 Feb 2013 13:50:04 GMTdescription changed
https://trac.sagemath.org/ticket/14058#comment:12
https://trac.sagemath.org/ticket/14058#comment:12
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/14058?action=diff&version=12">diff</a>)
</li>
</ul>
TicketSimonKingWed, 06 Feb 2013 14:47:15 GMT
https://trac.sagemath.org/ticket/14058#comment:13
https://trac.sagemath.org/ticket/14058#comment:13
<p>
In vanilla Sage, the coercion model stores (coercion) morphisms in its cache (which was a <code>TripleDict</code>, hence, with weak keys), and the only change introduced by your patch is to store weak references to the (coercion) morphism instead. Did I understand this correctly?
</p>
<p>
In vanilla Sage, the coercion model kept a strong reference to the morphism, which kept a strong reference to domain and codomain, which were thus not reclaimable, and so the item of the <code>TripleDict</code> has eternal life, in spite of the weak keys. Correct?
</p>
<p>
With the patch, the coercion model keeps a weak reference to the coercion morphism, hence, the strong reference from the morphism to domain and codomain doesn't matter, and thus the item of the <code>TripleDict</code> may disappear.
</p>
<p>
But how is the morphism kept alive? The coercion model only has a weak reference to it. Do I understand correctly that the morphism involved in the binary operation is, at the same time, stored in the coerce dict of its codomain? That's why it does not immediately die?
</p>
<p>
In other words, do I understand that the layout is as follows? We have parents A and B, and want to apply some binary operator, with a result in the parent C (C may coincide with either A or B). And we have maps r_A and r_B from A or B to C.
</p>
<p>
Both r_A and r_B are stored with a strong reference in a cache located in C, with weak keys A and B. At the same time, they are stored with a weak reference in the coercion model, again with weak keys A and B. r_A has strong references to C and to A, r_B has strong references to C and to B.
</p>
<p>
What do we want? Do we want that keeping C alive makes A and B survive? Or do we want that keeping both A and B alive makes C survive?
</p>
<p>
If the user has a strong reference to C, then C has a strong reference to r_A and r_B (in its coerce cache), and r_A/r_B strongly refers to A/B. Hence, the existence of C keeps A and B alive. Since C is a complicated object, it is more likely to be mortal, hence, probably it is not much of a problem.
</p>
<p>
If the user has a strong reference to both A and B, then C keeps a strong reference to both r_A and r_B, and the coercion model keeps a weak reference to r_A and r_B. Moreover, r_A and r_B strongly refer to C.
</p>
<p>
However, isn't it just a reference cycle between C and r_A/r_B? It would not prevent C from becoming collectable, right?
</p>
<p>
I doubt that we want that. It is not desirable that, when adding elements of <code>ZZ['x']</code> and QQ, the ring <code>QQ['x']</code> is created repeatedly (OK, polynomial rings have a strong cache anyway. But you see what I mean).
</p>
<p>
Or did I misunderstand something?
</p>
TicketSimonKingWed, 06 Feb 2013 15:59:01 GMTattachment set
https://trac.sagemath.org/ticket/14058
https://trac.sagemath.org/ticket/14058
<ul>
<li><strong>attachment</strong>
set to <em>testcoercion.py</em>
</li>
</ul>
<p>
An example that shows that the current patch needs work
</p>
TicketnbruinWed, 06 Feb 2013 15:59:06 GMT
https://trac.sagemath.org/ticket/14058#comment:14
https://trac.sagemath.org/ticket/14058#comment:14
<blockquote class="citation">
<p>
I doubt that we want that. It is not desirable that, when adding elements of <code>ZZ['x']</code> and QQ, the ring <code>QQ['x']</code> is created repeatedly (OK, polynomial rings have a strong cache anyway. But you see what I mean).
</p>
</blockquote>
<p>
In my opinion, that is exactly what we want, or at least what we have to settle for. If a person wants to work efficiently with <code>QQ['x']</code> he should ensure his elements already live there. Adding elements with the same parents is orders of magnitude faster than relying on coercion. A step closer is ensure that your parent remains in memory. I suspect that usually that will be the case, because if a user creates elements somewhere regularly he probably
</p>
<p>
In this scenario:
</p>
<pre class="wiki">P=[ZZ['x%d'%i] for i in [1..1000]]
F=[GF(p) for p in prime_range(1000)]
for Zxi in P:
for k in F:
a=P.0
b=F(1)
c=a+b
</pre><p>
If we keep the <code>GF(p)[xi]</code> alive because at some point we constructed an element in it from parents that are still around, we've just created quadratic memory requirements where linear should be enough.
</p>
<p>
I don't think that we can afford to assume that just because a user has combined two parents he/she will do so again in the future. We may be able to afford doing so if it happened <em>recently</em>, but that really is an optimisation, and currently I don't see a hook where we can do this efficiently.
</p>
<p>
A limited length buffer that gets updated whenever a certain component sees a parent go by might work. In Coercion <em>discovery</em> perhaps? That's already slow and it would exactly protect these automatically generated parents. Logically it's hard to defend but it may be OK in practice.
</p>
<p>
Another possibility is just <code>UniqueRepresentation_c.__cinit__</code>. Only when we're making a lot of parents do we care about losing them again ...
</p>
<p>
The problem with artificially keeping transient elements alive for longer is that you'll defeat the heuristics for generational garbage collection (which Python does), and we depend rather heavily on that to get rid of cyclic garbage. Testing would be required to see if this is indeed a problem, and it will be very application-dependent.
</p>
TicketSimonKingWed, 06 Feb 2013 16:02:35 GMT
https://trac.sagemath.org/ticket/14058#comment:15
https://trac.sagemath.org/ticket/14058#comment:15
<p>
Before posting a reply to Nils' post, here is an example that the existence of A and B does not ensure the survival of the pushout of A and B. First, attach <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14058/testcoercion.py" title="Attachment 'testcoercion.py' in Ticket #14058">testcoercion.py</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14058/testcoercion.py" title="Download"></a> into a Sage session. Then:
</p>
<pre class="wiki">sage: %attach /home/simon/SAGE/work/memleak/testcoercion.py
sage: OA = A()
sage: OB = B()
sage: cm = sage.structure.element.get_coercion_model()
sage: OC = cm.explain(OA,OB)
Coercion on left operand via
Conversion map:
From: A
To: C
Coercion on right operand via
Conversion map:
From: B
To: C
Arithmetic performed after coercions.
Result lives in C
sage: OC.is_coercion_cached(OA)
True
sage: OC.is_coercion_cached(OB)
True
sage: del OC
sage: import gc
sage: _ = gc.collect()
sage: len([X for X in gc.get_objects() if isinstance(X,C)])
0
sage: xc = OA(1)*OB(1)
sage: isinstance(xc.parent(),C)
True
sage: del xc
sage: _ = gc.collect()
sage: len([X for X in gc.get_objects() if isinstance(X,C)])
0
</pre><p>
I do believe that this is <em>not</em> desired behaviour.
</p>
TicketSimonKingWed, 06 Feb 2013 16:13:18 GMT
https://trac.sagemath.org/ticket/14058#comment:16
https://trac.sagemath.org/ticket/14058#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:14" title="Comment 14">nbruin</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
I doubt that we want that. It is not desirable that, when adding elements of <code>ZZ['x']</code> and QQ, the ring <code>QQ['x']</code> is created repeatedly (OK, polynomial rings have a strong cache anyway. But you see what I mean).
</p>
</blockquote>
<p>
In my opinion, that is exactly what we want, or at least what we have to settle for. If a person wants to work efficiently with <code>QQ['x']</code> he should ensure his elements already live there.
</p>
</blockquote>
<p>
Granted. But it is a very typical usage to <em>not</em> explicitly convert everything into a common parent, but let the coercion model do the work. That is what the coercion model is for!
</p>
<blockquote class="citation">
<blockquote>
<p>
I suspect that usually that will be the case, because if a user creates elements somewhere regularly he probably
</p>
</blockquote>
</blockquote>
<p>
However, in the example that I posted above, note that multiplying <code>OA(1)*OB(1)</code> repeatedly will also create <code>C()</code> <em>repeatedly</em>, even though it is a <code>UniqueRepresentation</code> and hence (weakly, nowadays) cached!
</p>
<p>
I think it is not acceptable that a multiple creation is triggered.
</p>
<blockquote class="citation">
<p>
In this scenario:
</p>
<pre class="wiki">P=[ZZ['x%d'%i] for i in [1..1000]]
F=[GF(p) for p in prime_range(1000)]
for Zxi in P:
for k in F:
a=P.0
b=F(1)
c=a+b
</pre><p>
If we keep the <code>GF(p)[xi]</code> alive because at some point we constructed an element in it from parents that are still around, we've just created quadratic memory requirements where linear should be enough.
</p>
</blockquote>
<p>
In your example, <code>GF(p)[xi]</code> is created exactly once, for each value of p and i. Clearly, in this situation it would not make sense to keep it alive, because it is never reused. However, I doubt that this is a typical use case.
</p>
<blockquote class="citation">
<p>
A limited length buffer that gets updated whenever a certain component sees a parent go by might work. In Coercion <em>discovery</em> perhaps? That's already slow and it would exactly protect these automatically generated parents. Logically it's hard to defend but it may be OK in practice.
</p>
<p>
Another possibility is just <code>UniqueRepresentation_c.__cinit__</code>. Only when we're making a lot of parents do we care about losing them again ...
</p>
</blockquote>
<p>
OK, but not all unique parent structures rely on it.
</p>
TicketnbruinWed, 06 Feb 2013 16:33:03 GMT
https://trac.sagemath.org/ticket/14058#comment:17
https://trac.sagemath.org/ticket/14058#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:16" title="Comment 16">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I think it is not acceptable that a multiple creation is triggered.
</p>
</blockquote>
<p>
...
Why not? Should the user expect that doing something a second time is faster than the first, without taking special precautions?
</p>
<blockquote class="citation">
<p>
In your example, <code>GF(p)[xi]</code> is created exactly once, for each value of p and i. Clearly, in this situation it would not make sense to keep it alive, because it is never reused. However, I doubt that this is a typical use case.
</p>
</blockquote>
<p>
But how do you tell the difference? Against too little caching there is a simple defense: keep a reference yourself. Against too much caching there is nothing you can do. You just run out of memory because data structures outside your control blow up.
</p>
<p>
We clearly have too much caching presently, in some cases by design. I think we first need a design that's fairly clean and for which we can reliably reason there's not "too much" caching (changing the order of memory requirement is definitely "too much" for me). Next step is tweaking it, possibly with MRU queues (which in that case should really be investigated for interfering with efficient GC, which is based on "objects either live very short or very long", so artificially creating objects with a medium lifespan is bad)
</p>
TicketSimonKingWed, 06 Feb 2013 17:10:57 GMT
https://trac.sagemath.org/ticket/14058#comment:18
https://trac.sagemath.org/ticket/14058#comment:18
<p>
By the way, it seems that the "insufficient" caching (not enough to keep C() alive) has no influence on performance, at least not if computations are done in a close cycle:
</p>
<pre class="wiki">sage: %attach /home/simon/SAGE/work/memleak/testcoercion.py
sage: OA = A()
sage: OB = B()
sage: a = OA(1)
sage: b = OB(1)
sage: def test(x,y):
....: for _ in xrange(2*10^5):
....: z = x*y
....:
sage: %time test(a,a)
CPU times: user 4.49 s, sys: 0.00 s, total: 4.49 s
Wall time: 4.49 s
sage: %time test(a,b)
CPU times: user 8.99 s, sys: 0.11 s, total: 9.11 s
Wall time: 9.13 s
sage: c = a*b
sage: print c.parent()
C
sage: %time test(a,b)
CPU times: user 8.85 s, sys: 0.03 s, total: 8.88 s
Wall time: 8.89 s
</pre><p>
In the first run of test(a,b), there is the possibility that <code>parent(a*b)</code> becomes created repeatedly, as I have demonstrated above. In the second run, <code>parent(a*b)</code> is kept alive, by keeping a pointer to the result of <code>a*b</code>. There is no significant difference in performance. Of course, we also see that multiplication within one parent is faster.
</p>
<p>
Is there a way to get the patchbots report significant regressions in some examples? Because I am sure that some parts of Sage (schemes and combinat are typical candidates) rely on a strong coercion cache.
</p>
TicketnbruinWed, 06 Feb 2013 17:45:31 GMT
https://trac.sagemath.org/ticket/14058#comment:19
https://trac.sagemath.org/ticket/14058#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:18" title="Comment 18">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
By the way, it seems that the "insufficient" caching (not enough to keep C() alive) has no influence on performance, at least not if computations are done in a close cycle:
</p>
</blockquote>
<p>
Ah right, because <code>C._coerce_from_hash[A]._codomain is C</code>, there's a cycle involving C so instead of getting deleted eagerly by Python's refcounting, it's only done with (relatively infrequent) GCs. So it even comes with a trick to improve this free "caching": Increase the interval for garbage collection. See <code>gc.get_threshold</code> and <code>gc.set_threshold</code>. By tweaking those numbers you can probably manipulate the running times in these examples.
</p>
<p>
<strong>NOTE:</strong> Doesn't <code>timeit</code> turn garbage collection off? So that may be a misleading tool for investigating performance for these things.
</p>
TicketSimonKingWed, 06 Feb 2013 19:01:10 GMT
https://trac.sagemath.org/ticket/14058#comment:20
https://trac.sagemath.org/ticket/14058#comment:20
<p>
I think it makes sense to ask <a class="ext-link" href="https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/7FCHou9nkIg"><span class="icon"></span>sage-devel</a> whether it is better to fix this leak or not.
</p>
<p>
I believe that keeping A and B alive should prevent C from garbage collection. In contrast, keeping C alive should <em>not</em> prevent A and B from garbage collection. Currently, it is the other way around. Perhaps we should try to think through whether having a weak reference to the domain of coercion or conversion maps would work.
</p>
TicketrobertwbWed, 06 Feb 2013 19:57:55 GMT
https://trac.sagemath.org/ticket/14058#comment:21
https://trac.sagemath.org/ticket/14058#comment:21
<p>
+1 to taking this discussion to sage-devel.
</p>
<p>
Patchbot: apply 14058-weak-binop-morphisms.patch trac_14058-doctest.patch
</p>
TicketSimonKingThu, 07 Feb 2013 12:21:03 GMT
https://trac.sagemath.org/ticket/14058#comment:22
https://trac.sagemath.org/ticket/14058#comment:22
<p>
The patchbot only reports one error, namely
</p>
<pre class="wiki">File "/home/patchbot/sage-5.7.beta3/devel/sage-14058/sage/rings/number_field/number_field_ideal.py", line 118:
sage: convert_to_idealprimedec_form(K, P)
Exception raised:
Traceback (most recent call last):
File "/home/patchbot/sage-5.7.beta3/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/home/patchbot/sage-5.7.beta3/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/home/patchbot/sage-5.7.beta3/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_2[5]>", line 1, in <module>
convert_to_idealprimedec_form(K, P)###line 118:
sage: convert_to_idealprimedec_form(K, P)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/rings/number_field/number_field_ideal.py", line 124, in convert_to_idealprimedec_form
K_bnf = gp(field.pari_bnf())
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 201, in __call__
return self._coerce_from_special_method(x)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 227, in _coerce_from_special_method
return (x.__getattribute__(s))(self)
File "sage_object.pyx", line 485, in sage.structure.sage_object.SageObject._gp_ (sage/structure/sage_object.c:4804)
File "sage_object.pyx", line 450, in sage.structure.sage_object.SageObject._interface_ (sage/structure/sage_object.c:4144)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 199, in __call__
return cls(self, x, name=name)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/expect.py", line 1280, in __init__
self._name = parent._create(value, name=name)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/interface.py", line 389, in _create
self.set(name, value)
File "/home/patchbot/sage-5.7.beta3/local/lib/python/site-packages/sage/interfaces/gp.py", line 511, in set
raise TypeError, "Error executing code in GP:\nCODE:\n\t%s\nPARI/GP ERROR:\n%s"%(cmd, out)
TypeError: Error executing code in GP:
CODE:
sage[14]=[[;], matrix(0,7), [;], Mat([0.E-38 + 3.52184386028273*I, 0.E-37 + 3.70366245659542*I, 0.E-37 + 2.91167081258838*I, 0.E-38 + 3.14159265358979*I, 0.E-38 + 9.04452675407645*I, 0.E-37 + 8.86270815776375*I, 0.E-37 + 9.65469980177079*I]), [[7, [-1, 2]~, 1, 1, [3, -2; 2, 1]], [13, [-5, 2]~, 1, 1, [-6, -2; 2, -8]], [19, [-3, 2]~, 1, 1, [5, -2; 2, 3]], [3, [1, 2]~, 2, 1, [1, 1; -1, 2]], [7, [3, 2]~, 1, 1, [-1, -2; 2, -3]], [13, [7, 2]~, 1, 1, [-5, -2; 2, -7]], [19, [5, 2]~, 1, 1, [-3, -2; 2, -5]]], 0, [y^2 + 3, [0, 1], -3, 2, [Mat([1, -0.500000000000000 - 0.866025403784439*I]), [1, -1.36602540378444; 1, 0.366025403784439], [1, -1; 1, 0], [2, -1; -1, -1], [3, 2; 0, 1], [1, -1; -1, -2], [3, [2, -1; 1, 1]]], [0.E-38 - 1.73205080756888*I], [1, 1/2*y - 1/2], [1, 1; 0, 2], [1, 0, 0, -1; 0, 1, 1, -1]], [[1, [], []], 1, 1, [6, -1/2*y + 1/2], []], [[;], [], []], [0, []]];
PARI/GP ERROR:
*** at top-level: sage[14]=[[;],matrix(0,7
*** ^--------------------
*** _[_]: not a vector.
*** Warning: precision too low for generators, not given.
</pre><p>
That's obscure to me. What does it mean?
</p>
TicketSimonKingThu, 07 Feb 2013 12:26:19 GMT
https://trac.sagemath.org/ticket/14058#comment:23
https://trac.sagemath.org/ticket/14058#comment:23
<p>
Unfortunately, I get
</p>
<pre class="wiki">sage -t "devel/sage-main/sage/rings/number_field/number_field_ideal.py"
[21.9 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 22.0 seconds
</pre><p>
So, what happened here?
</p>
<p>
In any case, I think the example of collectable pushout should be included in the documentation, combined with a warning to keep a pointer to the pushout (or avoid cross-parent arithmetic altogether), if speed matters.
</p>
<p>
I could write that. But to what location? Logically, it belongs to sage.structure.coerce, but nobody would read that. It might better be in a more general tutorial. What do you suggest?
</p>
TicketSimonKingThu, 07 Feb 2013 19:14:55 GMT
https://trac.sagemath.org/ticket/14058#comment:24
https://trac.sagemath.org/ticket/14058#comment:24
<p>
I did make ptest with a highly patched debug version of sage-5.7.beta2. Hence, I have the dependencies of <a class="closed ticket" href="https://trac.sagemath.org/ticket/13864" title="task: Configure Python with pydebug when SAGE_DEBUG is set (closed: fixed)">#13864</a> and additionally <a class="closed ticket" href="https://trac.sagemath.org/ticket/9107" title="defect: Nested class name mangling can be wrong in case of double nesting (closed: fixed)">#9107</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12951" title="enhancement: Support cached_methods on extension types (closed: fixed)">#12951</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/13916" title="defect: Fix inspection of interactive Cython code (closed: fixed)">#13916</a>. <a class="closed ticket" href="https://trac.sagemath.org/ticket/12313" title="defect: Fix yet another memory leak caused by caching of coercion data (closed: fixed)">#12313</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/14058" title="enhancement: Weakly reference binary operation codomains (closed: fixed)">#14058</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/13370" title="defect: Do not cache the result of is_Field externally (closed: fixed)">#13370</a> applied.
</p>
<p>
In the parallel test, I got two timeouts, namely in devel/sage/doc/en/bordeaux_2008/birds_other.rst and devel/sage/sage/interfaces/maxima.py. Running the former individually is long, but fine:
</p>
<pre class="wiki">sage -t -force_lib "devel/sage/doc/en/bordeaux_2008/birds_other.rst"
[339.8 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 339.9 seconds
</pre><p>
However, the maxima test is suspicious. I ran it twice individually. The first time I got an evil error:
</p>
<pre class="wiki">sage -t -force_lib "devel/sage/sage/interfaces/maxima.py"
**********************************************************************
File "/home/simon/SAGE/debug/sage-5.7.beta2/devel/sage/sage/interfaces/maxima.py", line 355:
sage: T.tlimit('n','inf')
Exception raised:
Traceback (most recent call last):
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_0[77]>", line 1, in <module>
T.tlimit('n','inf')###line 355:
sage: T.tlimit('n','inf')
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/interface.py", line 588, in __call__
return self._obj.parent().function_call(self._name, [self._obj] + list(args), kwds)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/interface.py", line 489, in function_call
return self.new(s)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/interface.py", line 264, in new
return self(code)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/interface.py", line 199, in __call__
return cls(self, x, name=name)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/maxima.py", line 1153, in __init__
ExpectElement.__init__(self, parent, value, is_name=False, name=None)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/expect.py", line 1280, in __init__
self._name = parent._create(value, name=name)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/interface.py", line 389, in _create
self.set(name, value)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/maxima.py", line 998, in set
self._eval_line(cmd)
File "/home/simon/SAGE/debug/sage-5.7.beta2/local/lib/python/site-packages/sage/interfaces/maxima.py", line 754, in _eval_line
assert line_echo.strip() == line.strip()
AssertionError
**********************************************************************
1 items had failures:
1 of 97 in __main__.example_0
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/simon/.sage//tmp/maxima_30383.py
[13.7 s]
----------------------------------------------------------------------
The following tests failed:
sage -t -force_lib "devel/sage/sage/interfaces/maxima.py"
Total time for all tests: 13.8 seconds
</pre><p>
and the second time I got
</p>
<pre class="wiki">sage -t -force_lib "devel/sage/sage/interfaces/maxima.py"
[12.6 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 12.6 seconds
</pre><p>
Is the error something we should worry about? Or is that likely to be the effect of a neutrino hitting my laptop?
</p>
TicketSimonKingTue, 26 Feb 2013 19:45:04 GMT
https://trac.sagemath.org/ticket/14058#comment:25
https://trac.sagemath.org/ticket/14058#comment:25
<p>
At <a class="closed ticket" href="https://trac.sagemath.org/ticket/14159" title="defect: Don't install callbacks on values of TripleDict, MonoDict (closed: fixed)">#14159</a>, I am providing a version of <code>MonoDict</code> and <code>TripleDict</code> that optionally allows to store the values by weak references. Provided that the performance is OK, Nils Bruin suggests to consider to use this here, hence, make <a class="closed ticket" href="https://trac.sagemath.org/ticket/14159" title="defect: Don't install callbacks on values of TripleDict, MonoDict (closed: fixed)">#14159</a> a dependency. What do you think?
</p>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/14058#comment:26
https://trac.sagemath.org/ticket/14058#comment:26
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
TicketSimonKingThu, 15 Aug 2013 07:23:46 GMT
https://trac.sagemath.org/ticket/14058#comment:27
https://trac.sagemath.org/ticket/14058#comment:27
<p>
Note to myself: Work on this ticket...
</p>
TicketSimonKingSat, 16 Nov 2013 23:44:01 GMT
https://trac.sagemath.org/ticket/14058#comment:28
https://trac.sagemath.org/ticket/14058#comment:28
<p>
Lost track again.
</p>
<p>
I tried to import the patches and turn them into git branches. It mostly worked, but I had to resolve some conflicts with the second patch.
</p>
<p>
Would it be OK to change this hg-ticket into a git-ticket?
</p>
TicketSimonKingSun, 17 Nov 2013 14:45:39 GMTstatus changed; reviewer, work_issues set
https://trac.sagemath.org/ticket/14058#comment:29
https://trac.sagemath.org/ticket/14058#comment:29
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
set to <em>Simon King</em>
</li>
<li><strong>work_issues</strong>
set to <em>Fix doctests; decide between hg and git</em>
</li>
</ul>
<p>
While I had to resolve conflicts when importing the two patches into the git master branch, the patches do apply to sage-5.12.beta5, and the patchbot reports the same on sage-5.13.beta2. So, perhaps it is OK to keep this a mercurial ticket.
</p>
<p>
However, with the gitification of the patches, I find (similar to the patchbot)
</p>
<pre class="wiki">sage -t --long /mnt/storage2TB/patchbot/Sage/sage-5.13.beta2/devel/sage/sage/rings/polynomial/plural.pyx # 17 doctests failed
sage -t --long /mnt/storage2TB/patchbot/Sage/sage-5.13.beta2/devel/sage/sage/libs/singular/function.pyx # 1 doctest failed
</pre><p>
The patchbot additionally finds
</p>
<pre class="wiki">sage -t --long /mnt/storage2TB/patchbot/Sage/sage-5.13.beta2/devel/sage/sage/algebras/free_algebra.py # 1 doctest failed
sage -t --long /mnt/storage2TB/patchbot/Sage/sage-5.13.beta2/devel/sage/sage/structure/coerce.pyx # 1 doctest failed
</pre><p>
So, this requires some work.
</p>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/14058#comment:30
https://trac.sagemath.org/ticket/14058#comment:30
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/14058#comment:31
https://trac.sagemath.org/ticket/14058#comment:31
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
Ticketvbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/14058#comment:32
https://trac.sagemath.org/ticket/14058#comment:32
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketSimonKingThu, 16 Jul 2015 19:55:56 GMT
https://trac.sagemath.org/ticket/14058#comment:33
https://trac.sagemath.org/ticket/14058#comment:33
<p>
I plan to create a branch for it. Probably it will fix <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/18905" title="defect: fix more leaks found in #18897 (needs_work)">#18905</a>.
</p>
TicketSimonKingThu, 16 Jul 2015 22:39:48 GMTbranch set
https://trac.sagemath.org/ticket/14058#comment:34
https://trac.sagemath.org/ticket/14058#comment:34
<ul>
<li><strong>branch</strong>
set to <em>u/SimonKing/weakly_reference_binary_operation_codomains</em>
</li>
</ul>
TicketSimonKingThu, 16 Jul 2015 22:46:19 GMTwork_issues changed; commit set
https://trac.sagemath.org/ticket/14058#comment:35
https://trac.sagemath.org/ticket/14058#comment:35
<ul>
<li><strong>commit</strong>
set to <em>b9141eef1042c8b8fb1179b30844d5bdb737b96e</em>
</li>
<li><strong>work_issues</strong>
changed from <em>Fix doctests; decide between hg and git</em> to <em>Fix doctests</em>
</li>
</ul>
<p>
Here is a branch. The merge was not trivial, so, hopefully I did it well.
</p>
<p>
Next, we have to see if the "old problem" (i.e., <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/14058/testcoercion.py" title="Attachment 'testcoercion.py' in Ticket #14058">attachment:testcoercion.py</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/14058/testcoercion.py" title="Download"></a>) is still problematic, and of course there may be doctest failures.
</p>
<p>
At least, the issue from <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:20" title="Comment 20">comment:20</a> is solved: We do have weak references to the domain of coercion maps now.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ac68d9b2d7624d32dc885f42bbd83d372fa8da5f"><span class="icon"></span>ac68d9b</a></td><td><code>Remove strong references to parents used in binary operations in the coercion model.</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e153d0813221d3681030fbf24bbbc283d3717162"><span class="icon"></span>e153d08</a></td><td><code>#14058: Add doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b9141eef1042c8b8fb1179b30844d5bdb737b96e"><span class="icon"></span>b9141ee</a></td><td><code>Merge branch 'ticket/14058' into develop</code>
</td></tr></table>
TicketSimonKingFri, 17 Jul 2015 07:43:00 GMT
https://trac.sagemath.org/ticket/14058#comment:36
https://trac.sagemath.org/ticket/14058#comment:36
<pre class="wiki">sage -t src/sage/algebras/commutative_dga.py # 3 doctests failed
sage -t src/sage/algebras/free_algebra.py # 1 doctest failed
sage -t src/sage/rings/polynomial/plural.pyx # 17 doctests failed
sage -t src/sage/dev/sagedev.py # 2 doctests failed
sage -t src/sage/structure/coerce.pyx # 2 doctests failed
sage -t src/sage/libs/singular/function.pyx # 2 doctests failed
</pre><p>
Not bad (and the sagedev error very likely is because I have rerere enabled, hence, it is unrelated anyway).
</p>
TicketSimonKingFri, 17 Jul 2015 08:06:34 GMT
https://trac.sagemath.org/ticket/14058#comment:37
https://trac.sagemath.org/ticket/14058#comment:37
<p>
Details:
</p>
<pre class="wiki">sage -t src/sage/libs/singular/function.pyx
**********************************************************************
File "src/sage/libs/singular/function.pyx", line 413, in sage.libs.singular.function.is_singular_poly_wrapper
Failed example:
H.<x,y,z> = A.g_algebra({z*x:x*z+2*x, z*y:y*z-2*y})
Expected nothing
Got:
Exception KeyError: (The ring pointer 0x7f7c7470c8d0,) in 'sage.libs.singular.ring.singular_ring_delete' ignored
**********************************************************************
File "src/sage/libs/singular/function.pyx", line 1250, in sage.libs.singular.function.SingularFunction.__call__
Failed example:
I = Ideal(I.groebner_basis())
Expected nothing
Got:
Exception KeyError: (The ring pointer 0x7f7c7470ca40,) in 'sage.libs.singular.ring.singular_ring_delete' ignored
**********************************************************************
</pre><p>
--> It seems that the cache became too weak. All errors in <code>sage -t src/sage/rings/polynomial/plural.pyx</code> and <code>sage -t src/sage/algebras/commutative_dga.py</code> are of that form.
</p>
<pre class="wiki">sage -t src/sage/structure/coerce.pyx
**********************************************************************
File "src/sage/structure/coerce.pyx", line 418, in sage.structure.coerce.CoercionModel_cache_maps.get_cache
Failed example:
print copy(left_morphism_ref())
Exception raised:
Traceback (most recent call last):
File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.structure.coerce.CoercionModel_cache_maps.get_cache[4]>", line 1, in <module>
print copy(left_morphism_ref())
NameError: name 'left_morphism_ref' is not defined
**********************************************************************
File "src/sage/structure/coerce.pyx", line 422, in sage.structure.coerce.CoercionModel_cache_maps.get_cache
Failed example:
print right_morphism_ref
Exception raised:
Traceback (most recent call last):
File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 496, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/king/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 858, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.structure.coerce.CoercionModel_cache_maps.get_cache[5]>", line 1, in <module>
print right_morphism_ref
NameError: name 'right_morphism_ref' is not defined
**********************************************************************
</pre><p>
--> I got something wrong in my merge.
</p>
<p>
So, we actually just have two errors.
</p>
TicketgitFri, 17 Jul 2015 15:29:58 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:38
https://trac.sagemath.org/ticket/14058#comment:38
<ul>
<li><strong>commit</strong>
changed from <em>b9141eef1042c8b8fb1179b30844d5bdb737b96e</em> to <em>90ed181ae9df152e99a652636a7e9dc29b466916</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=90ed181ae9df152e99a652636a7e9dc29b466916"><span class="icon"></span>90ed181</a></td><td><code>Trivial fix for a coercion doctest</code>
</td></tr></table>
TicketSimonKingFri, 17 Jul 2015 21:46:53 GMT
https://trac.sagemath.org/ticket/14058#comment:39
https://trac.sagemath.org/ticket/14058#comment:39
<p>
<a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a> is probably related. Perhaps (if we can finish the work here) <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a> can eventually be closed as a duplicate.
</p>
TicketSimonKingFri, 17 Jul 2015 21:52:57 GMT
https://trac.sagemath.org/ticket/14058#comment:40
https://trac.sagemath.org/ticket/14058#comment:40
<p>
In the following example
</p>
<pre class="wiki"> sage: import gc
sage: from sage.rings.polynomial.plural import NCPolynomialRing_plural
sage: from sage.algebras.free_algebra import FreeAlgebra
sage: A1.<x,y,z> = FreeAlgebra(QQ, 3)
sage: R1 = A1.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A2.<x,y,z> = FreeAlgebra(GF(5), 3)
sage: R2 = A2.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A3.<x,y,z> = FreeAlgebra(GF(11), 3)
sage: R3 = A3.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: A4.<x,y,z> = FreeAlgebra(GF(13), 3)
sage: R4 = A4.g_algebra({y*x:x*y-z, z*x:x*z+2*x, z*y:y*z-2*y}, order=TermOrder('degrevlex', 2))
sage: _ = gc.collect()
sage: foo = R1.gen(0)
sage: del foo
sage: del R1
sage: _ = gc.collect()
sage: del R2
sage: _ = gc.collect()
sage: del R3
sage: _ = gc.collect()
</pre><p>
the reference counting goes wrong after each <code>gc.collect()</code>.
</p>
TicketgitFri, 17 Jul 2015 22:38:01 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:41
https://trac.sagemath.org/ticket/14058#comment:41
<ul>
<li><strong>commit</strong>
changed from <em>90ed181ae9df152e99a652636a7e9dc29b466916</em> to <em>64b572ccf6403be0c617d0e39de4b1e6cd5dbc58</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=64b572ccf6403be0c617d0e39de4b1e6cd5dbc58"><span class="icon"></span>64b572c</a></td><td><code>refcount libsingular rings used in plural</code>
</td></tr></table>
TicketSimonKingFri, 17 Jul 2015 22:40:10 GMT
https://trac.sagemath.org/ticket/14058#comment:42
https://trac.sagemath.org/ticket/14058#comment:42
<p>
Got it! When a non-commutative ring is created, it was forgotten to put the underlying libsingular ring into Sage's refcounting system. Should be fixed now. I'll see if <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a> is fixed now, too.
</p>
TicketSimonKingFri, 17 Jul 2015 22:50:36 GMTstatus, author changed; work_issues deleted
https://trac.sagemath.org/ticket/14058#comment:43
https://trac.sagemath.org/ticket/14058#comment:43
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>Fix doctests</em> deleted
</li>
<li><strong>author</strong>
changed from <em>Robert Bradshaw</em> to <em>Robert Bradshaw, Nils Bruin</em>
</li>
</ul>
<p>
Sigh. <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a> is not fixed, as the following example shows.
</p>
<pre class="wiki">sage: import gc
sage: _ = gc.collect()
sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
sage: from sage.libs.singular.groebner_strategy import GroebnerStrategy
sage: n = len([x for x in gc.get_objects() if isinstance(x,MPolynomialRing_libsingular)])
sage: P = MPolynomialRing_libsingular(GF(541), 2, ('x', 'y'), TermOrder('degrevlex', 2))
sage: strat = GroebnerStrategy(Ideal([P.gen(0) + P.gen(1)]))
sage: del strat
sage: del P
sage: _ = gc.collect()
sage: n == len([x for x in gc.get_objects() if isinstance(x,MPolynomialRing_libsingular)])
False
</pre><p>
I'd suggest to proceed as follows. Provided that all tests pass, I can review Robert's and Nils' patches, someone else reviews my additions (which are the merging and the latest commit), and then we make this a dependency for <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a>.
</p>
TicketgitSat, 18 Jul 2015 06:18:17 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:44
https://trac.sagemath.org/ticket/14058#comment:44
<ul>
<li><strong>commit</strong>
changed from <em>64b572ccf6403be0c617d0e39de4b1e6cd5dbc58</em> to <em>9793dbc9bc64bfdce43ff1e626453816c302696d</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9793dbc9bc64bfdce43ff1e626453816c302696d"><span class="icon"></span>9793dbc</a></td><td><code>Make one test more stable</code>
</td></tr></table>
TicketSimonKingSat, 18 Jul 2015 06:26:18 GMT
https://trac.sagemath.org/ticket/14058#comment:45
https://trac.sagemath.org/ticket/14058#comment:45
<p>
Sometimes, the following test in sage/categories/fields.py has failed:
</p>
<pre class="wiki"> sage: import gc
sage: _ = gc.collect()
sage: n = len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)])
sage: for i in prime_range(100):
....: R = ZZ.quotient(i)
....: t = R in Fields()
sage: _ = gc.collect()
sage: len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)]) - n
1
</pre><p>
The reason for the failure: Sometimes the last line gave 0, not 1. It is not totally clear to me why the different results occurred. But in fact 0 is better than 1 here (one additional ring got collected). So, I hope my change is OK.
</p>
TicketSimonKingSat, 18 Jul 2015 06:45:34 GMT
https://trac.sagemath.org/ticket/14058#comment:46
https://trac.sagemath.org/ticket/14058#comment:46
<p>
I am giving a positive review to Robert's code and Nils' doctest. I consider my contribution too small to make me author, but I think my changes need a review.
</p>
TicketpbruinMon, 20 Jul 2015 14:40:49 GMT
https://trac.sagemath.org/ticket/14058#comment:47
https://trac.sagemath.org/ticket/14058#comment:47
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:45" title="Comment 45">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Sometimes, the following test in sage/categories/fields.py has failed:
</p>
<pre class="wiki"> sage: import gc
sage: _ = gc.collect()
sage: n = len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)])
sage: for i in prime_range(100):
....: R = ZZ.quotient(i)
....: t = R in Fields()
sage: _ = gc.collect()
sage: len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)]) - n
1
</pre><p>
The reason for the failure: Sometimes the last line gave 0, not 1. It is not totally clear to me why the different results occurred. But in fact 0 is better than 1 here (one additional ring got collected).
</p>
</blockquote>
<p>
Shouldn't the result always be 1 given that <code>R</code> refers to <code>Zmod(97)</code> after the loop (unless there is a surviving reference from a <code>Zmod(97)</code> created in some earlier test)? What happens if you do <code>del R</code> before the last <code>gc.collect()</code>?
</p>
TicketSimonKingMon, 20 Jul 2015 14:56:52 GMT
https://trac.sagemath.org/ticket/14058#comment:48
https://trac.sagemath.org/ticket/14058#comment:48
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:47" title="Comment 47">pbruin</a>:
</p>
<blockquote class="citation">
<p>
Shouldn't the result always be 1 given that <code>R</code> refers to <code>Zmod(97)</code> after the loop (unless there is a surviving reference from a <code>Zmod(97)</code> created in some earlier test)?
</p>
</blockquote>
<p>
Exactly. And what's strange: It isn't reproducible. Sometimes I get 1, sometimes I get 0. And I always get 1 when I run it on the command line.
</p>
<blockquote class="citation">
<blockquote>
<p>
What happens if you do <code>del R</code> before the last <code>gc.collect()</code>?
</p>
</blockquote>
</blockquote>
<p>
No idea.
</p>
TicketSimonKingMon, 20 Jul 2015 15:02:30 GMT
https://trac.sagemath.org/ticket/14058#comment:49
https://trac.sagemath.org/ticket/14058#comment:49
<p>
To be precise: I saw this problem precisely once. When I run the previous test about 20 times, it always passes.
</p>
TicketgitTue, 21 Jul 2015 13:18:48 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:50
https://trac.sagemath.org/ticket/14058#comment:50
<ul>
<li><strong>commit</strong>
changed from <em>9793dbc9bc64bfdce43ff1e626453816c302696d</em> to <em>2c04029f9a194bd87befa0cd99016e2f267a836f</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2c04029f9a194bd87befa0cd99016e2f267a836f"><span class="icon"></span>2c04029</a></td><td><code>make a test against a memory leak clearer/more stable</code>
</td></tr></table>
TicketSimonKingTue, 21 Jul 2015 13:19:18 GMT
https://trac.sagemath.org/ticket/14058#comment:51
https://trac.sagemath.org/ticket/14058#comment:51
<p>
Hopefully the new version of the test is reliable.
</p>
TicketchapotonTue, 04 Aug 2015 07:37:21 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:52
https://trac.sagemath.org/ticket/14058#comment:52
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
excerpt from patchbot report:
</p>
<pre class="wiki">+inside file: b/src/sage/structure/coerce.pyx
+@@ -1265,33 +1287,74 @@ cdef class CoercionModel_cache_maps(CoercionModel):
++ raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map"
++ raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map"
+Old-style raise statement inserted on 2 non-empty lines
</pre>
TicketSimonKingTue, 04 Aug 2015 07:59:42 GMT
https://trac.sagemath.org/ticket/14058#comment:53
https://trac.sagemath.org/ticket/14058#comment:53
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:52" title="Comment 52">chapoton</a>:
</p>
<blockquote class="citation">
<p>
excerpt from patchbot report:
</p>
<pre class="wiki">+inside file: b/src/sage/structure/coerce.pyx
+@@ -1265,33 +1287,74 @@ cdef class CoercionModel_cache_maps(CoercionModel):
++ raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map"
++ raise RuntimeError, "BUG in coercion model: coerce_map_from must return a Map"
+Old-style raise statement inserted on 2 non-empty lines
</pre></blockquote>
<p>
Actually this was not inserted, but moved. Would it be possible for you to change this to new-style raise statement by a reviewer commit?
</p>
TicketchapotonTue, 04 Aug 2015 08:57:55 GMTcommit, branch changed
https://trac.sagemath.org/ticket/14058#comment:54
https://trac.sagemath.org/ticket/14058#comment:54
<ul>
<li><strong>commit</strong>
changed from <em>2c04029f9a194bd87befa0cd99016e2f267a836f</em> to <em>8713960717f916d459e0b6a0dcef391dc992d9b2</em>
</li>
<li><strong>branch</strong>
changed from <em>u/SimonKing/weakly_reference_binary_operation_codomains</em> to <em>public/ticket/14058</em>
</li>
</ul>
<p>
done. I also did it for some of the other raise in the same file.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e976b797736452bc9fc07e85c0fc1ffcd5d07e88"><span class="icon"></span>e976b79</a></td><td><code>Merge branch 'u/SimonKing/weakly_reference_binary_operation_codomains' into 6.9.b0</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8713960717f916d459e0b6a0dcef391dc992d9b2"><span class="icon"></span>8713960</a></td><td><code>trac #14058 convert some raise statements into py3 syntax</code>
</td></tr></table>
TicketchapotonTue, 04 Aug 2015 08:58:07 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:55
https://trac.sagemath.org/ticket/14058#comment:55
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketSimonKingThu, 06 Aug 2015 14:06:37 GMT
https://trac.sagemath.org/ticket/14058#comment:56
https://trac.sagemath.org/ticket/14058#comment:56
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:46" title="Comment 46">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I am giving a positive review to Robert's code and Nils' doctest. I consider my contribution too small to make me author, but I think my changes need a review.
</p>
</blockquote>
<p>
Could someone please finish the review? The patchbot thinks the branch is fine.
</p>
TicketjpfloriThu, 06 Aug 2015 14:11:50 GMTstatus, reviewer, description changed
https://trac.sagemath.org/ticket/14058#comment:57
https://trac.sagemath.org/ticket/14058#comment:57
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Simon King</em> to <em>Simon King, Frédéric Chapoton, Jean-Pierre Flori</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/14058?action=diff&version=57">diff</a>)
</li>
</ul>
<p>
Your change is small and makes sense.
</p>
TicketvdelecroixMon, 10 Aug 2015 09:56:54 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:58
https://trac.sagemath.org/ticket/14058#comment:58
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
From the patchbot
</p>
<pre class="wiki">sage -t --long src/sage/structure/coerce.pyx
**********************************************************************
File "src/sage/structure/coerce.pyx", line 1307, in sage.structure.coerce.CoercionModel_cache_maps.coercion_maps
Failed example:
print N2-N0
Expected:
0
Got:
-1
</pre><p>
Is it what we should get?
</p>
TicketvdelecroixMon, 10 Aug 2015 09:58:06 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:59
https://trac.sagemath.org/ticket/14058#comment:59
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:58" title="Comment 58">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
From the patchbot
...
</p>
</blockquote>
<p>
wrong ticket... should be <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/18905" title="defect: fix more leaks found in #18897 (needs_work)">#18905</a>. Sorry for the noise.
</p>
TicketSimonKingMon, 10 Aug 2015 11:53:55 GMT
https://trac.sagemath.org/ticket/14058#comment:60
https://trac.sagemath.org/ticket/14058#comment:60
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:59" title="Comment 59">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:58" title="Comment 58">vdelecroix</a>:
</p>
<blockquote class="citation">
<p>
From the patchbot
...
</p>
</blockquote>
<p>
wrong ticket... should be <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/18905" title="defect: fix more leaks found in #18897 (needs_work)">#18905</a>. Sorry for the noise.
</p>
</blockquote>
<p>
The patchbot HERE does report this problem. So, what do you mean that it "should be <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/18905" title="defect: fix more leaks found in #18897 (needs_work)">#18905</a>"?
</p>
TicketvdelecroixMon, 10 Aug 2015 11:55:17 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:61
https://trac.sagemath.org/ticket/14058#comment:61
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Right! I confused myself.
</p>
TicketvdelecroixMon, 10 Aug 2015 12:08:30 GMT
https://trac.sagemath.org/ticket/14058#comment:62
https://trac.sagemath.org/ticket/14058#comment:62
<p>
The failing doctest is reproducible after applying this branch onto <code>6.9.beta1</code>.
</p>
TicketslabbeTue, 11 Aug 2015 18:43:21 GMT
https://trac.sagemath.org/ticket/14058#comment:63
https://trac.sagemath.org/ticket/14058#comment:63
<p>
On sage-6.9.beta1, I get:
</p>
<pre class="wiki">$ sage nbruin.sage
Time: CPU 7.95 s, Wall: 7.95 s
Time: CPU 7.68 s, Wall: 7.68 s
Time: CPU 76.76 s, Wall: 76.94 s
</pre><p>
On sage-6.9.beta1 + <a class="closed ticket" href="https://trac.sagemath.org/ticket/14058" title="enhancement: Weakly reference binary operation codomains (closed: fixed)">#14058</a>, I get a very good improvement:
</p>
<pre class="wiki">$ sage nbruin.sage
Time: CPU 2.48 s, Wall: 2.48 s
Time: CPU 2.07 s, Wall: 2.07 s
Time: CPU 32.38 s, Wall: 32.42 s
</pre><p>
Note: I originally thought it <a class="ext-link" href="https://groups.google.com/d/msg/sage-devel/n7_21tCO4zI/cuCAlO01DwAJ"><span class="icon"></span>was a regression in sage</a> since I was getting this earlier this morning with sage-6.8 + <a class="closed ticket" href="https://trac.sagemath.org/ticket/14058" title="enhancement: Weakly reference binary operation codomains (closed: fixed)">#14058</a>. But thanksfully, it seems to be a improvement obtained from this ticket.
</p>
<p>
The file <code>nbruin.sage</code> is from <a class="ext-link" href="http://trac.sagemath.org/ticket/14058#comment:4"><span class="icon"></span>comment 4</a>:
</p>
<pre class="wiki"> $ cat nbruin.sage
a=ZZ(1)
b=QQ(1)
c=ZZ['x'](1)
d=b+c
def test(x,y):
for i in xrange(10^6):
_=x+y
time test(a,b)
time test(a,c)
time test(b,c)
</pre>
TicketslabbeTue, 11 Aug 2015 19:01:02 GMT
https://trac.sagemath.org/ticket/14058#comment:64
https://trac.sagemath.org/ticket/14058#comment:64
<p>
I also get improvements on tests above of Simon King at <a class="ext-link" href="http://trac.sagemath.org/ticket/14058#comment:18"><span class="icon"></span>comments 18</a>.
</p>
<p>
With sage-6.9.beta1:
</p>
<pre class="wiki">$ sage testcoercion.sage
Time: CPU 1.69 s, Wall: 1.69 s
Time: CPU 4.05 s, Wall: 4.05 s
C
Time: CPU 4.03 s, Wall: 4.03 s
</pre><p>
With sage-6.9.beta1 + this ticket:
</p>
<pre class="wiki">$ sage testcoercion.sage
Time: CPU 0.34 s, Wall: 0.34 s
Time: CPU 0.85 s, Wall: 0.85 s
C
Time: CPU 0.86 s, Wall: 0.86 s
</pre><p>
(My file testcoercion.sage contains testcoercion.py + the lines at comment 18.)
</p>
TicketslabbeWed, 12 Aug 2015 09:30:06 GMT
https://trac.sagemath.org/ticket/14058#comment:65
https://trac.sagemath.org/ticket/14058#comment:65
<p>
With sage-6.9.beta1 + this ticket, I get <code>All tests passed</code> when I do
</p>
<pre class="wiki">./sage -t --long src/sage/structure/coerce.pyx
</pre><p>
but I get the problem when I test after the build
</p>
<pre class="wiki">./sage -bt --long src/sage/structure/coerce.pyx
</pre><p>
which gives the error:
</p>
<pre class="wiki">**********************************************************************
File "src/sage/structure/coerce.pyx", line 1307, in sage.structure.coerce.CoercionModel_cache_maps.coercion_maps
Failed example:
print N2-N0
Expected:
0
Got:
-1
**********************************************************************
</pre><p>
I have never seen such a behavior.
</p>
TicketgitWed, 12 Aug 2015 11:08:38 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:66
https://trac.sagemath.org/ticket/14058#comment:66
<ul>
<li><strong>commit</strong>
changed from <em>8713960717f916d459e0b6a0dcef391dc992d9b2</em> to <em>b73d5372cd889d8b5e72e0ac56a781ba4dde9761</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c136ebd10a5ec7d45c7855a2e2ea66b3a23ce65e"><span class="icon"></span>c136ebd</a></td><td><code>Merge #14058 into 6.9.beta1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b73d5372cd889d8b5e72e0ac56a781ba4dde9761"><span class="icon"></span>b73d537</a></td><td><code>Trac #14058: fix broken doctest</code>
</td></tr></table>
TicketgitWed, 12 Aug 2015 11:41:01 GMTcommit changed
https://trac.sagemath.org/ticket/14058#comment:67
https://trac.sagemath.org/ticket/14058#comment:67
<ul>
<li><strong>commit</strong>
changed from <em>b73d5372cd889d8b5e72e0ac56a781ba4dde9761</em> to <em>13cb2a78983efb8f4edb00d6d10f8fc7da62e80d</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e153d0813221d3681030fbf24bbbc283d3717162"><span class="icon"></span>e153d08</a></td><td><code>#14058: Add doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b9141eef1042c8b8fb1179b30844d5bdb737b96e"><span class="icon"></span>b9141ee</a></td><td><code>Merge branch 'ticket/14058' into develop</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=90ed181ae9df152e99a652636a7e9dc29b466916"><span class="icon"></span>90ed181</a></td><td><code>Trivial fix for a coercion doctest</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=64b572ccf6403be0c617d0e39de4b1e6cd5dbc58"><span class="icon"></span>64b572c</a></td><td><code>refcount libsingular rings used in plural</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=9793dbc9bc64bfdce43ff1e626453816c302696d"><span class="icon"></span>9793dbc</a></td><td><code>Make one test more stable</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2c04029f9a194bd87befa0cd99016e2f267a836f"><span class="icon"></span>2c04029</a></td><td><code>make a test against a memory leak clearer/more stable</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e976b797736452bc9fc07e85c0fc1ffcd5d07e88"><span class="icon"></span>e976b79</a></td><td><code>Merge branch 'u/SimonKing/weakly_reference_binary_operation_codomains' into 6.9.b0</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8713960717f916d459e0b6a0dcef391dc992d9b2"><span class="icon"></span>8713960</a></td><td><code>trac #14058 convert some raise statements into py3 syntax</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c136ebd10a5ec7d45c7855a2e2ea66b3a23ce65e"><span class="icon"></span>c136ebd</a></td><td><code>Merge #14058 into 6.9.beta1</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=13cb2a78983efb8f4edb00d6d10f8fc7da62e80d"><span class="icon"></span>13cb2a7</a></td><td><code>Trac #14058: fix broken doctest</code>
</td></tr></table>
TicketslabbeWed, 12 Aug 2015 11:46:22 GMT
https://trac.sagemath.org/ticket/14058#comment:68
https://trac.sagemath.org/ticket/14058#comment:68
<p>
In other doctests in the same file, <code>GF(3)</code>, <code>GF(7)</code> and <code>GF(41)</code> are also created. While there are references to <code>GF(7)</code> and <code>GF(41)</code>, no variable references to <code>GF(3)</code> which is available for gargage collection. If <code>GF(3)</code> gets garbage collected before the offending doctest, then everything is fine, the doctest pass. But it can also be garbage collected in the call to <code>gc.collect()</code> in the doctest. If this is the case, then <code>N0</code> was also counting <code>GF(3)</code> which explains why <code>N2-N0</code> can be negative.
</p>
<p>
One way to fix this is to call <code>gc.collect()</code> before creating <code>N0</code>.
</p>
<p>
(my first fix was using sets of ids... sorry for the noise).
</p>
TicketSimonKingWed, 12 Aug 2015 12:08:19 GMT
https://trac.sagemath.org/ticket/14058#comment:69
https://trac.sagemath.org/ticket/14058#comment:69
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/14058#comment:68" title="Comment 68">slabbe</a>:
</p>
<blockquote class="citation">
<p>
One way to fix this is to call <code>gc.collect()</code> before creating <code>N0</code>.
</p>
</blockquote>
<p>
I agree, that's the way to fix it.
</p>
TicketslabbeWed, 12 Aug 2015 12:16:19 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:70
https://trac.sagemath.org/ticket/14058#comment:70
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketjpfloriThu, 13 Aug 2015 09:31:47 GMTstatus changed
https://trac.sagemath.org/ticket/14058#comment:71
https://trac.sagemath.org/ticket/14058#comment:71
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
I assume Simon's comment gives positive review here as this was the only thing to fix.
</p>
TicketjpfloriThu, 13 Aug 2015 09:32:20 GMTreviewer changed
https://trac.sagemath.org/ticket/14058#comment:72
https://trac.sagemath.org/ticket/14058#comment:72
<ul>
<li><strong>reviewer</strong>
changed from <em>Simon King, Frédéric Chapoton, Jean-Pierre Flori</em> to <em>Simon King, Frédéric Chapoton, Jean-Pierre Flori, Sébastien Labbé</em>
</li>
</ul>
TicketchapotonFri, 14 Aug 2015 06:57:02 GMTmilestone changed
https://trac.sagemath.org/ticket/14058#comment:73
https://trac.sagemath.org/ticket/14058#comment:73
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.4</em> to <em>sage-6.9</em>
</li>
</ul>
TicketvbraunSat, 12 Sep 2015 09:56:59 GMTdependencies deleted
https://trac.sagemath.org/ticket/14058#comment:74
https://trac.sagemath.org/ticket/14058#comment:74
<ul>
<li><strong>dependencies</strong>
<em>#12313</em> deleted
</li>
</ul>
TicketvbraunSun, 13 Sep 2015 22:12:27 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/14058#comment:75
https://trac.sagemath.org/ticket/14058#comment:75
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>public/ticket/14058</em> to <em>13cb2a78983efb8f4edb00d6d10f8fc7da62e80d</em>
</li>
</ul>
TicketjdemeyerThu, 17 Sep 2015 19:59:56 GMTcommit deleted
https://trac.sagemath.org/ticket/14058#comment:76
https://trac.sagemath.org/ticket/14058#comment:76
<ul>
<li><strong>commit</strong>
<em>13cb2a78983efb8f4edb00d6d10f8fc7da62e80d</em> deleted
</li>
</ul>
<p>
On arando:
</p>
<pre class="wiki">
sage -t --long src/sage/categories/fields.py
**********************************************************************
File "src/sage/categories/fields.py", line 104, in sage.categories.fields.Fields.__contains__
Failed example:
len([X for X in gc.get_objects() if isinstance(X, sage.rings.finite_rings.integer_mod_ring.IntegerModRing_generic)]) - n
Expected:
0
Got:
-1
**********************************************************************
</pre>
TicketjdemeyerSat, 19 Sep 2015 10:53:54 GMT
https://trac.sagemath.org/ticket/14058#comment:77
https://trac.sagemath.org/ticket/14058#comment:77
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/19244" title="defect: Broken doctest in src/sage/categories/fields.py (closed: fixed)">#19244</a>
</p>
Ticket