Sage: Ticket #12876: Fix element and parent classes of Hom categories to be abstract, and simplify the Hom logic.
https://trac.sagemath.org/ticket/12876
<p>
This patch fixes the parent and element classes for Hom categories to
be purely abstract, and simplifies the Hom logic:
</p>
<ul><li>Unified the logic for selecting the class when building a Homset
(e.g. Homset, <a class="missing wiki">RingHomset?</a>, <a class="missing wiki">HeckeModuleHomspace?</a>, ...). This is now
systematically done through the _Hom_ hook. The logic still has a
fundamental flaw, but that's for the later <a class="closed ticket" href="https://trac.sagemath.org/ticket/10668" title="defect: Refactor category support for morphisms (Hom is not a functorial ... (closed: fixed)">#10668</a>.
</li><li>The cache for Hom is handled at a single point in Hom
In particular, homsets created via the _Hom_ hook are now unique.
</li><li>If category is None, Hom simply calls itself with the meet of the
categories of the parent, which removes a cache handling duplication.
</li><li>Parent.Hom calls Hom directly (removes duplicate _Hom_ logic).
</li><li><a class="missing wiki">ParentWithBase?</a>.Hom was redundant and is gone.
</li><li>Reduce the footprint of the current trick to delegate
Hom(F,F)(on_basis=...) to module_morphism, allow for the diagonal
option too, an make sure the homset category is set properly.
</li><li>Update a doctest in sage.modules.vector_space_homspace to take into
account that homsets created via _Hom_ are now unique.
</li><li>Scheme is (apparently) an abstract base class; so it should not be
instantiated. I changed some doctests in
sage.schemes.generic.<a class="missing wiki">SchemeMorphism?</a> to use instead the concrete
Spec(ZZ). Those doctests were breaking because Scheme does not
implement equality, which is required for Hom caching.
</li></ul><p>
As a byproduct, the <a class="missing wiki">HeckeModules?</a> category does not import any more
<a class="missing wiki">HeckeModulesHomspace?</a>, which was a recurrent source of import loops.
</p>
<p>
<a class="closed ticket" href="https://trac.sagemath.org/ticket/11935" title="enhancement: Make parent/element classes independent of base rings (closed: fixed)">#11935</a> depends on this ticket
</p>
<p>
Apply:
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classskrel11521.patch" title="Attachment 'trac_12876_categoryfix_abstract_classskrel11521.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classskrel11521.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classskrel11521.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876reviewer.patch" title="Attachment 'trac_12876reviewer.patch' in Ticket #12876">trac_12876reviewer.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876reviewer.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a>
</li></ul>enusSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/12876
Trac 1.1.6nthieryTue, 24 Apr 2012 21:57:55 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:1
https://trac.sagemath.org/ticket/12876#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
Note: this might depend on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>
</p>
TicketnthieryTue, 24 Apr 2012 21:58:44 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:2
https://trac.sagemath.org/ticket/12876#comment:2
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=2">diff</a>)
</li>
</ul>
TicketnthieryTue, 24 Apr 2012 22:11:07 GMTreviewer set
https://trac.sagemath.org/ticket/12876#comment:3
https://trac.sagemath.org/ticket/12876#comment:3
<ul>
<li><strong>reviewer</strong>
set to <em>Simon King</em>
</li>
</ul>
<p>
Note: there are two doctests failures that I don't know how to handle, related to the refcounting of Singular rings::
</p>
<pre class="wiki">
sage t devel/sagecombinat/sage/rings/polynomial/multi_polynomial_libsingular.pyx
**********************************************************************
File "/opt/sage5.0.beta11/devel/sagecombinat/sage/rings/polynomial/multi_polynomial_libsingular.pyx", line 418:
sage: len(ring_refcount_dict) == n
Expected:
True
Got:
False
sage t devel/sagecombinat/sage/libs/singular/ring.pyx
**********************************************************************
File "/opt/sage5.0.beta11/devel/sagecombinat/sage/libs/singular/ring.pyx", line 469:
sage: ring_ptr in ring_refcount_dict
Expected:
False
Got:
True
</pre><p>
Help fixing those welcome!
</p>
TicketnthieryWed, 25 Apr 2012 00:13:14 GMT
https://trac.sagemath.org/ticket/12876#comment:4
https://trac.sagemath.org/ticket/12876#comment:4
<p>
The updated patch includes two small related improvements I had in later patches. I moved them here to resolve the conflict.
</p>
TicketnthieryWed, 25 Apr 2012 00:13:50 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:5
https://trac.sagemath.org/ticket/12876#comment:5
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=5">diff</a>)
</li>
</ul>
TicketnthieryWed, 25 Apr 2012 00:37:44 GMT
https://trac.sagemath.org/ticket/12876#comment:6
https://trac.sagemath.org/ticket/12876#comment:6
<p>
Oops. The updated patch includes two further hunks I had forgotten.
</p>
TicketnthieryWed, 25 Apr 2012 00:46:43 GMT
https://trac.sagemath.org/ticket/12876#comment:7
https://trac.sagemath.org/ticket/12876#comment:7
<p>
Arr, yet another missing hunk ... Time to go to bed!
</p>
TicketnthieryWed, 25 Apr 2012 13:53:52 GMTdependencies set
https://trac.sagemath.org/ticket/12876#comment:8
https://trac.sagemath.org/ticket/12876#comment:8
<ul>
<li><strong>dependencies</strong>
set to <em>#12877</em>
</li>
</ul>
<p>
For the record, all test passed on 5.0.beta13, with <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a> applied (and a couple unrelated ones). I expect no dependency upon <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, but can't swear at this point.
</p>
TicketnthieryWed, 25 Apr 2012 13:55:16 GMTcc changed
https://trac.sagemath.org/ticket/12876#comment:9
https://trac.sagemath.org/ticket/12876#comment:9
<ul>
<li><strong>cc</strong>
<em>SimonKing</em> added
</li>
</ul>
TicketnthieryWed, 25 Apr 2012 14:23:44 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:10
https://trac.sagemath.org/ticket/12876#comment:10
<ul>
<li><strong>dependencies</strong>
changed from <em>#12877</em> to <em>#12875, #12877</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:8" title="Comment 8">nthiery</a>:
</p>
<blockquote class="citation">
<p>
I expect no dependency upon <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, but can't swear at this point.
</p>
</blockquote>
<p>
Actually it does depend on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>.
</p>
TicketSimonKingThu, 26 Apr 2012 16:38:29 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/12876#comment:11
https://trac.sagemath.org/ticket/12876#comment:11
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>Fix ring_refcount_dict problem</em>
</li>
</ul>
<p>
With <a class="closed ticket" href="https://trac.sagemath.org/ticket/12808" title="enhancement: Optimize ClassCallMetaClass using Cython (closed: fixed)">#12808</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a> applied on top of sage5.1.notebook, all doctests pass. But when adding the patch from here, there are two problems:
</p>
<pre class="wiki"> sage t force_lib "devel/sage/sage/libs/singular/ring.pyx"
sage t force_lib "devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx"
</pre><p>
Namely:
</p>
<pre class="wiki">sage t force_lib "devel/sage/sage/libs/singular/ring.pyx"
**********************************************************************
File "/mnt/local/king/SAGE/experiment/notebook/sage5.1.notebook/devel/sage/sage/libs/singular/ring.pyx", line 469:
sage: ring_ptr in ring_refcount_dict
Expected:
False
Got:
True
**********************************************************************
</pre><p>
and
</p>
<pre class="wiki">sage t force_lib "devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx"
**********************************************************************
File "/mnt/local/king/SAGE/experiment/notebook/sage5.1.notebook/devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx", line 418:
sage: len(ring_refcount_dict) == n
Expected:
True
Got:
False
**********************************************************************
</pre><p>
So, apparently it is only a single problem.
</p>
TicketSimonKingThu, 26 Apr 2012 16:40:56 GMT
https://trac.sagemath.org/ticket/12876#comment:12
https://trac.sagemath.org/ticket/12876#comment:12
<p>
PS: I just verified that the problem also occurs without <a class="closed ticket" href="https://trac.sagemath.org/ticket/12808" title="enhancement: Optimize ClassCallMetaClass using Cython (closed: fixed)">#12808</a>.
</p>
TicketnthieryThu, 26 Apr 2012 17:08:48 GMT
https://trac.sagemath.org/ticket/12876#comment:13
https://trac.sagemath.org/ticket/12876#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:11" title="Comment 11">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
With <a class="closed ticket" href="https://trac.sagemath.org/ticket/12808" title="enhancement: Optimize ClassCallMetaClass using Cython (closed: fixed)">#12808</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a> applied on top of sage5.1.notebook, all doctests pass. But when adding the patch from here, there are two problems:
</p>
</blockquote>
<p>
Yeah, I know (see comment 3 above). I feel quite stuck with those
failures, and would really appreciate if you could investigate them
since you know much better than me the caching for Singular rings.
</p>
<p>
Thanks!
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingFri, 27 Apr 2012 05:13:58 GMT
https://trac.sagemath.org/ticket/12876#comment:14
https://trac.sagemath.org/ticket/12876#comment:14
<p>
In one of my "weak caching" patches (see <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>), I also had to fix a ring_refcount_dict test. Thus, I tried to see whether applying <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> in addition to your patch helps. Unfortunately there is an incompatibility, so, it is not resolved yet.
</p>
TicketSimonKingFri, 27 Apr 2012 07:17:24 GMT
https://trac.sagemath.org/ticket/12876#comment:15
https://trac.sagemath.org/ticket/12876#comment:15
<p>
Since there is a positive review for <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> (which belong together and have otherwise no pending dependency), could you consider to rebase your patch relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>?
</p>
TicketSimonKingFri, 27 Apr 2012 07:21:20 GMT
https://trac.sagemath.org/ticket/12876#comment:16
https://trac.sagemath.org/ticket/12876#comment:16
<p>
PS: I am just attempting a rebase, to see whether <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> fixes the ring_refcount_dict problem.
</p>
TicketSimonKingFri, 27 Apr 2012 07:31:42 GMT
https://trac.sagemath.org/ticket/12876#comment:17
https://trac.sagemath.org/ticket/12876#comment:17
<p>
Here is the rejection:
</p>
<div class="wikicode"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>homset.py</a>
</h2>
<table class="tracdiff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File homset.py">
</th>
<th title="File homset.py">
</th>
<td><em> def Hom(X, Y, category=None):</em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>165</th><th>203</th><td class="l"><span> if H.domain() is X and H.codomain() is Y:</span></td>
</tr><tr>
<th>166</th><th>204</th><td class="l"><span> return H</span></td>
</tr><tr>
<th>167</th><th>205</th><td class="l"><span></span></td>
</tr>
</tbody><tbody class="rem">
<tr class="first">
<th>168</th><th> </th><td class="l"><del> try:</del></td>
</tr><tr>
<th>169</th><th> </th><td class="l"><del> return X._Hom_(Y, category)</del></td>
</tr><tr>
<th>170</th><th> </th><td class="l"><del> except (AttributeError, TypeError):</del></td>
</tr><tr>
<th>171</th><th> </th><td class="l"><del> pass</del></td>
</tr><tr class="last">
<th>172</th><th> </th><td class="l"><del></del></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>173</th><th>206</th><td class="l"><span> cat_X = X.category()</span></td>
</tr><tr>
<th>174</th><th>207</th><td class="l"><span> cat_Y = Y.category()</span></td>
</tr><tr>
<th>175</th><th>208</th><td class="l"><span> if category is None:</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>176</th><th> </th><td class="l"><span> category = cat_X._meet_(cat_Y)</span></td>
</tr><tr>
<th>177</th><th> </th><td class="l"><span> elif isinstance(category, Category):</span></td>
</tr><tr>
<th>178</th><th> </th><td class="l"><span> if not cat_X.is_subcategory(category):</span></td>
</tr><tr>
<th>179</th><th> </th><td class="l"><span> raise TypeError, "%s is not in %s"%(X, category)</span></td>
</tr><tr>
<th>180</th><th> </th><td class="l"><span> if not cat_Y.is_subcategory(category):</span></td>
</tr><tr>
<th>181</th><th> </th><td class="l"><span> raise TypeError, "%s is not in %s"%(Y, category)</span></td>
</tr><tr>
<th>182</th><th> </th><td class="l"><span> else:</span></td>
</tr>
<tr>
<th> </th><th>209</th><td class="r"><span> return Hom(X,Y,category=cat_X._meet_(cat_Y))</span></td>
</tr><tr class="last">
<th> </th><th>210</th><td class="r"><span> if not isinstance(category, Category):</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>183</th><th>211</th><td class="l"><span> raise TypeError, "Argument category (= %s) must be a category."%category</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>184</th><th> </th><td class="l"><span> # Now, as the category may have changed, we try to find the hom set in the cache, again:</span></td>
</tr><tr>
<th>185</th><th> </th><td class="l"><span> key = (X,Y,category)</span></td>
</tr><tr>
<th>186</th><th> </th><td class="l"><span> if _cache.has_key(key):</span></td>
</tr><tr>
<th>187</th><th> </th><td class="l"><span> H = _cache[key]()</span></td>
</tr><tr>
<th>188</th><th> </th><td class="l"><span> if H:</span></td>
</tr><tr>
<th>189</th><th> </th><td class="l"><span> # Are domain or codomain breaking the unique parent condition?</span></td>
</tr><tr>
<th>190</th><th> </th><td class="l"><span> if H.domain() is X and H.codomain() is Y:</span></td>
</tr><tr>
<th>191</th><th> </th><td class="l"><span> return H</span></td>
</tr><tr>
<th>192</th><th> </th><td class="l"><span></span></td>
</tr><tr>
<th>193</th><th> </th><td class="l"><span> # coercing would be incredibly annoying, since the domain and codomain</span></td>
</tr><tr>
<th>194</th><th> </th><td class="l"><span> # are totally different objects</span></td>
</tr><tr>
<th>195</th><th> </th><td class="l"><span> #X = category(X); Y = category(Y)</span></td>
</tr>
<tr>
<th> </th><th>212</th><td class="r"><span> if not cat_X.is_subcategory(category):</span></td>
</tr><tr>
<th> </th><th>213</th><td class="r"><span> raise TypeError, "%s is not in %s"%(X, category)</span></td>
</tr><tr>
<th> </th><th>214</th><td class="r"><span> if not cat_Y.is_subcategory(category):</span></td>
</tr><tr class="last">
<th> </th><th>215</th><td class="r"><span> raise TypeError, "%s is not in %s"%(Y, category)</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>196</th><th>216</th><td class="l"><span></span></td>
</tr><tr>
<th>197</th><th>217</th><td class="l"><span> # construct H</span></td>
</tr><tr>
<th>198</th><th>218</th><td class="l"><span> # Design question: should the Homset classes get the category or the homset category?</span></td>
</tr><tr>
<th>199</th><th>219</th><td class="l"><span> # For the moment, this is the category, for compatibility with the current implementations</span></td>
</tr><tr>
<th>200</th><th>220</th><td class="l"><span> # of Homset in rings, schemes, ...</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>201</th><th> </th><td class="l"><span> H = category.hom_category().parent_class(X, Y, category = category)</span></td>
</tr><tr>
<th>202</th><th> </th><td class="l"><span> </span></td>
</tr><tr>
<th>203</th><th> </th><td class="l"><span> ##_cache[key] = weakref.ref(H)</span></td>
</tr><tr>
<th>204</th><th> </th><td class="l"><span> _cache[(X, Y, category)] = weakref.ref(H)</span></td>
</tr>
<tr>
<th> </th><th>221</th><td class="r"><span> from sets_cat import Sets</span></td>
</tr><tr>
<th> </th><th>222</th><td class="r"><span> try:</span></td>
</tr><tr>
<th> </th><th>223</th><td class="r"><span> H = X._Hom_(Y, category)</span></td>
</tr><tr>
<th> </th><th>224</th><td class="r"><span> except (AttributeError, TypeError):</span></td>
</tr><tr>
<th> </th><th>225</th><td class="r"><span> H = Homset(X, Y, category = category)</span></td>
</tr><tr class="last">
<th> </th><th>226</th><td class="r"><span> _cache[key] = weakref.ref(H)</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>205</th><th>227</th><td class="l"><span> return H</span></td>
</tr><tr>
<th>206</th><th>228</th><td class="l"><span></span></td>
</tr><tr>
<th>207</th><th>229</th><td class="l"><span>def hom(X, Y, f):</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
Why do you import Sets (new line 221)?
</p>
<p>
Do I understand correctly: If Hom receives None as category, then you determine the category as the meet of the categories of domain and codomain, and call Hom again. Did you test that the calling overhead does not matter?
</p>
TicketSimonKingFri, 27 Apr 2012 07:38:46 GMT
https://trac.sagemath.org/ticket/12876#comment:18
https://trac.sagemath.org/ticket/12876#comment:18
<p>
Another point: I had inserted a comment in <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, namely "Apparently _Hom_ is supposed to be cached." This is why I did not insert stuff in the (weak) cache.
</p>
<p>
Do you lift that assumption? Then, we should see what _Hom_ methods actually do caching. They should at most do <em>weak</em> caching.
</p>
TicketSimonKingFri, 27 Apr 2012 07:47:43 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_categoryfix_abstract_classntrel11521.patch</em>
</li>
</ul>
<p>
Nicolas' patch rebased rel <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>
</p>
TicketSimonKingFri, 27 Apr 2012 08:01:55 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:19
https://trac.sagemath.org/ticket/12876#comment:19
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=19">diff</a>)
</li>
</ul>
<p>
I have rebased your patch relative to <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> (as I said: This already has positive review).
</p>
<p>
Please have a look whether you are happy with my changes: I only changed sage/categories/homset.py, by using the :trac: directive in two places, fixing one hunk that has only applied with fuzz and removing the import of Sets. Apart from that, I adopted your logic of constructing the homset.
</p>
<p>
So far, I have tested sage/categories/homset.py and have verified that the previously failing tests are indeed fixed by <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> (in fact, I also had some other pathes applied, but I am convinced that #<a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> suffices). I am now running the full tests suite, with <em>only</em> <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> (and its dependency <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>) and the new patch (and its dependencies <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a>).
</p>
<p>
Apply trac_12876_categoryfix_abstract_classntrel11521.patch
</p>
TicketSimonKingFri, 27 Apr 2012 11:40:40 GMT
https://trac.sagemath.org/ticket/12876#comment:20
https://trac.sagemath.org/ticket/12876#comment:20
<p>
Hm. With sage5.0.beta13 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Download"></a>, I get one error, namely a segfault in
</p>
<pre class="wiki">sage t force_lib "devel/sage/sage/rings/number_field/number_field_rel.py"
</pre><p>
But strange enough: When I ran it with verbose, all tests passed, and the segfault came from leaving sage. That reminds me another problem: The deallocation of the (unique) pari instance is not done as it is supposed to be. This is fixed by the second patch of <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a>. Hence, I am trying again, with <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> added.
</p>
TicketSimonKingFri, 27 Apr 2012 11:43:30 GMT
https://trac.sagemath.org/ticket/12876#comment:21
https://trac.sagemath.org/ticket/12876#comment:21
<p>
Yes! Adding <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> fixes the segfault. I am now running doctests. Do you mind adding it as another dependency, provided that the test suite passes?
</p>
TicketSimonKingFri, 27 Apr 2012 13:58:58 GMTdependencies, description changed
https://trac.sagemath.org/ticket/12876#comment:22
https://trac.sagemath.org/ticket/12876#comment:22
<ul>
<li><strong>dependencies</strong>
changed from <em>#12875, #12877</em> to <em>#715, #11521, #12875, #12877, #12215</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=22">diff</a>)
</li>
</ul>
<p>
With sage5.0.beta13 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Download"></a>, all doc tests pass.
</p>
<p>
I made some additional cosmetic changes in a reviewer patch.
</p>
<p>
Further questions:
</p>
<p>
In sage/category/hecke_modules.py, you refer to ticket number ??? for fixing _test_zero and _test_elements. In sage/schemes/elliptic_curves/ell_curve_isogeny.py, you leave a reference to trac open as well. Have the tickets now been created? I didn't fix that yet.
</p>
<p>
I see that you rename <code>Rings.HomCategory.ParentMethods.__new__</code> into <code>__new__bx</code>. Does that mean the ugly <code>__new__</code> method ought to be removed (finally!)?
But then, shouldn't <code>__getnewargs__</code> be removed as well?
</p>
<p>
Please tell whether you want to address these questions in an additional patch. And I hope you agree with adding <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> to the dependencies? Only <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> needs review (hint...).
</p>
<p>
If you do agree, then put this to <em>positive review</em>, please.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classntrel11521.patch trac_12876reviewer.patch
</p>
TicketSimonKingFri, 27 Apr 2012 14:00:57 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876reviewer.patch</em>
</li>
</ul>
TicketSimonKingFri, 27 Apr 2012 14:02:48 GMT
https://trac.sagemath.org/ticket/12876#comment:23
https://trac.sagemath.org/ticket/12876#comment:23
<p>
Sorry, I made a lastminute change in the reviewer patch (namely, by mistake, I changed "ticket ???" into "ticket 12876", but I think ??? really refers to a ticket that has not been created, yet. Anyway, feel free to change ??? into something real! And disregard trac_12876reviewer.2.patch, which was created by mistake.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classntrel11521.patch trac_12876reviewer.patch
</p>
TicketSimonKingFri, 27 Apr 2012 16:20:17 GMT
https://trac.sagemath.org/ticket/12876#comment:24
https://trac.sagemath.org/ticket/12876#comment:24
<p>
I tried whether one can remove the <code>__new__</code> and <code>__getnewargs__</code> methods. However, when doing so, one gets a reproducible timeout in the tests for sage/rings/polynomial/multi_polynomial_sequence.py  that's the only error.
</p>
<p>
So, better keep it as it is.
</p>
<p>
Recall: I would give it a positive review, if you agree with the additional dependencies.
</p>
TicketnthieryFri, 27 Apr 2012 17:14:02 GMT
https://trac.sagemath.org/ticket/12876#comment:25
https://trac.sagemath.org/ticket/12876#comment:25
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:17" title="Comment 17">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Do I understand correctly: If Hom receives None as category, then you determine the category as the meet of the categories of domain and codomain, and call Hom again. Did you test that the calling overhead does not matter?
</p>
</blockquote>
<p>
I did not test it. But it simplifies quite much the logic and the calling overhead occurs only for the first call, so I assumed it was negligible.
</p>
TicketnthieryFri, 27 Apr 2012 17:17:22 GMT
https://trac.sagemath.org/ticket/12876#comment:26
https://trac.sagemath.org/ticket/12876#comment:26
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:21" title="Comment 21">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Yes! Adding <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> fixes the segfault. I am now running doctests. Do you mind adding it as another dependency, provided that the test suite passes?
</p>
</blockquote>
<p>
That's ok for me. Thanks for tracking all of those down! I'll try to review <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a>, but that probably won't be before Monday.
</p>
TicketnthieryFri, 27 Apr 2012 17:25:51 GMT
https://trac.sagemath.org/ticket/12876#comment:27
https://trac.sagemath.org/ticket/12876#comment:27
<blockquote>
<p>
Hi Simon!
</p>
</blockquote>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:22" title="Comment 22">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
With sage5.0.beta13 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> and <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521.patch" title="Download"></a>, all doc tests pass.
</p>
</blockquote>
<p>
Cool!
</p>
<blockquote class="citation">
<p>
In sage/category/hecke_modules.py, you refer to ticket number ??? for fixing _test_zero and _test_elements. In sage/schemes/elliptic_curves/ell_curve_isogeny.py, you leave a reference to trac open as well. Have the tickets now been created? I didn't fix that yet.
</p>
</blockquote>
<p>
Oops. Those are <a class="closed ticket" href="https://trac.sagemath.org/ticket/12879" title="defect: TestSuite failures for HeckeModule Homsets (closed: fixed)">#12879</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a> respectively.
</p>
<blockquote class="citation">
<p>
I see that you rename <code>Rings.HomCategory.ParentMethods.__new__</code> into <code>__new__bx</code>. Does that mean the ugly <code>__new__</code> method ought to be removed (finally!)?
But then, shouldn't <code>__getnewargs__</code> be removed as well?
</p>
</blockquote>
<p>
Yes, that was my intention indeed: in principle they should not be needed, and I forgot to remove them. I'll investigate the failure you report on Monday, unless you beat me to it.
</p>
<blockquote class="citation">
<p>
Please tell whether you want to address these questions in an additional patch.
</p>
</blockquote>
<p>
Unless removing <span class="underline">getnewargs</span> turns out to be hard, I'd rather fix all of the above right now.
Feel free to experiment: I won't download the patches before Monday.
</p>
<p>
Thanks a lot for all your quick work on this :)
</p>
TicketnthieryFri, 27 Apr 2012 17:31:19 GMT
https://trac.sagemath.org/ticket/12876#comment:28
https://trac.sagemath.org/ticket/12876#comment:28
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:18" title="Comment 18">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Another point: I had inserted a comment in <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, namely "Apparently _Hom_ is supposed to be cached." This is why I did not insert stuff in the (weak) cache.
</p>
<p>
Do you lift that assumption? Then, we should see what _Hom_ methods actually do caching. They should at most do <em>weak</em> caching.
</p>
</blockquote>
<p>
Yes: I consider that all the caching logic should be in Hom; in particular the _Hom_ methods should not do caching (and if I recall correctly, they currently don't). We should probably add a note about this in the documentation of Hom.
</p>
TicketnthieryMon, 30 Apr 2012 08:19:43 GMT
https://trac.sagemath.org/ticket/12876#comment:29
https://trac.sagemath.org/ticket/12876#comment:29
<p>
I have uploaded a reviewer's patch which fixes the trac ticket numbers, and removes completely Rings.<a class="missing wiki">HomCategory?</a>. I could not reproduce the timeout:
</p>
<pre class="wiki">sage t "devel/sagecombinat/sage/rings/polynomial/multi_polynomial_sequence.py"
[8.6 s]
</pre><p>
Can you check again?
</p>
<p>
If you are happy with the change, please fold everything, and set a positive review.
</p>
TicketSimonKingMon, 30 Apr 2012 10:46:45 GMT
https://trac.sagemath.org/ticket/12876#comment:30
https://trac.sagemath.org/ticket/12876#comment:30
<p>
I just verified: I don't see a timeout anymore. Strange.
</p>
<p>
Anyway, I am now running make testlong, and if everything works, I'll give it a positive review and provide a combined patch.
</p>
TicketSimonKingMon, 30 Apr 2012 13:04:40 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:31
https://trac.sagemath.org/ticket/12876#comment:31
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Too bad. "make testlong" resulted in
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/categories/map.pyx"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 382:
sage: phi.category()
Expected:
Category of hom sets in Category of rings
Got:
Category of hom sets in Category of sets
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 388:
sage: f.category()
Expected:
Join of Category of hom sets in Category of rings and Category of hom sets in Category of modules over Rational Field
Got:
Category of hom sets in Category of modules over Rational Field
**********************************************************************
</pre><p>
and
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/categories/homset.py"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/homset.py", line 228:
sage: Hom(PA,PJ).category()
Expected:
Join of Category of hom sets in Category of rings and Category of hom sets in Category of modules over Rational Field
Got:
Category of hom sets in Category of modules over Rational Field
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/homset.py", line 339:
sage: End(QQ).category()
Expected:
Category of hom sets in Category of rings
Got:
Category of hom sets in Category of sets
</pre><p>
I have sage5.0.beta13 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12876" title="enhancement: Fix element and parent classes of Hom categories to be abstract, and ... (closed: fixed)">#12876</a>.
</p>
TicketSimonKingMon, 30 Apr 2012 13:06:44 GMT
https://trac.sagemath.org/ticket/12876#comment:32
https://trac.sagemath.org/ticket/12876#comment:32
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:31" title="Comment 31">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I have sage5.0.beta13 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12875" title="defect: Fix the homset category initialization for ModularAbelianVariety's ... (closed: fixed)">#12875</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12877" title="enhancement: Categories for padics, schemes, and some more rings (closed: fixed)">#12877</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/12876" title="enhancement: Fix element and parent classes of Hom categories to be abstract, and ... (closed: fixed)">#12876</a>.
</p>
</blockquote>
<p>
To be precise (in case I forgot to apply one patch):
</p>
<pre class="wiki">
king@mpc622:/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage$ hg qa
trac_715_combined.patch
trac_11521_homset_weakcache_combined.patch
trac_12875categoryfix_abvar_homspacent.patch
trac_12877categoryfor_more_rings_and_schemesnt.patch
trac12215_weak_cached_function.patch
trac12215_segfault_fixes.patch
trac_12876_categoryfix_abstract_classntrel11521.patch
trac_12876reviewer.patch
trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch
</pre>
TicketnthieryMon, 30 Apr 2012 13:34:49 GMT
https://trac.sagemath.org/ticket/12876#comment:33
https://trac.sagemath.org/ticket/12876#comment:33
<p>
Ok, this is totally benign. Now, to minimize change, the easiest is probably to keep the <a class="missing wiki">HomCategory?</a> even if we delete its containt. The updated reviewer's patch does just that.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketnthieryMon, 30 Apr 2012 13:35:44 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</em>
</li>
</ul>
TicketSimonKingWed, 02 May 2012 10:58:50 GMTwork_issues changed
https://trac.sagemath.org/ticket/12876#comment:34
https://trac.sagemath.org/ticket/12876#comment:34
<ul>
<li><strong>work_issues</strong>
changed from <em>Fix ring_refcount_dict problem</em> to <em>Fix the reviewnt patch</em>
</li>
</ul>
<p>
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a> doesn't work, because you confused ` with '.
</p>
<p>
Please change the following hunk:
</p>
<div class="wikicode"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>hecke_modules.py</a>
</h2>
<table class="tracdiff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File hecke_modules.py">
</th>
<th title="File hecke_modules.py">
</th>
<td><em> class HeckeModules(Category_module):</em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>116</th><th>116</th><td class="l"><span> Fixing :meth:`_test_zero` (``__call__`` should accept a</span></td>
</tr><tr>
<th>117</th><th>117</th><td class="l"><span> function as input) and :meth:`_test_elements` (modular</span></td>
</tr><tr>
<th>118</th><th>118</th><td class="l"><span> form morphisms elements should inherit from categories) is</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>119</th><th> </th><td class="l"><span> :trac:`<del>???</del>`.</span></td>
</tr>
<tr class="last">
<th> </th><th>119</th><td class="r"><span> :trac:`<ins>12879</ins>`.</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>120</th><th>120</th><td class="l"><span></span></td>
</tr><tr>
<th>121</th><th>121</th><td class="l"><span> TESTS::</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
since the tobechanged file has <code>:trac:'???'</code>, hence, no backticks around ???.
</p>
TicketSimonKingWed, 02 May 2012 13:13:50 GMTwork_issues changed
https://trac.sagemath.org/ticket/12876#comment:35
https://trac.sagemath.org/ticket/12876#comment:35
<ul>
<li><strong>work_issues</strong>
changed from <em>Fix the reviewnt patch</em> to <em>Fix the reviewnt patch, fix doctests in map.pyx and homset.py</em>
</li>
</ul>
<p>
I get four errors with <code>make ptestlong</code>:
</p>
<pre class="wiki">
The following tests failed:
sage t long force_lib devel/sage/sage/categories/homset.py # 2 doctests failed
sage t long force_lib devel/sage/sage/categories/map.pyx # 2 doctests failed

</pre><p>
namely
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/categories/homset.py"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/homset.py", line 228:
sage: Hom(PA,PJ).category()
Expected:
Join of Category of hom sets in Category of rings and Category of hom sets in Category of modules over Rational Field
Got:
Category of hom sets in Category of modules over Rational Field
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/homset.py", line 337:
sage: End(QQ).category()
Expected:
Category of hom sets in Category of rings
Got:
Category of hom sets in Category of sets
**********************************************************************
</pre><p>
and
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/categories/map.pyx"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 382:
sage: phi.category()
Expected:
Category of hom sets in Category of rings
Got:
Category of hom sets in Category of sets
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 388:
sage: f.category()
Expected:
Join of Category of hom sets in Category of rings and Category of hom sets in Category of modules over Rational Field
Got:
Category of hom sets in Category of modules over Rational Field
**********************************************************************
</pre><p>
For reference, I have
</p>
<pre class="wiki">king@mpc622:/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage$ hg qapplied
trac_12808classcall_speedupfh.patch
trac_12808_nested_class_cython.patch
trac_12808classcall_cdef.patch
trac12215_weak_cached_function.patch
trac12215_segfault_fixes.patch
trac_715_combined.patch
trac_11521_homset_weakcache_combined.patch
trac_12875categoryfix_abvar_homspacent.patch
trac_12877categoryfor_more_rings_and_schemesnt.patch
trac_12876_categoryfix_abstract_classntrel11521.patch
trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch
</pre><p>
How is that different from your setting?
</p>
TicketnthieryWed, 02 May 2012 13:39:23 GMT
https://trac.sagemath.org/ticket/12876#comment:36
https://trac.sagemath.org/ticket/12876#comment:36
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:34" title="Comment 34">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a> doesn't work, because you confused ` with '.
</p>
<p>
Please change the following hunk:
</p>
<div class="wikicode"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>hecke_modules.py</a>
</h2>
<table class="tracdiff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File hecke_modules.py">
</th>
<th title="File hecke_modules.py">
</th>
<td><em> class HeckeModules(Category_module):</em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>116</th><th>116</th><td class="l"><span> Fixing :meth:`_test_zero` (``__call__`` should accept a</span></td>
</tr><tr>
<th>117</th><th>117</th><td class="l"><span> function as input) and :meth:`_test_elements` (modular</span></td>
</tr><tr>
<th>118</th><th>118</th><td class="l"><span> form morphisms elements should inherit from categories) is</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>119</th><th> </th><td class="l"><span> :trac:`<del>???</del>`.</span></td>
</tr>
<tr class="last">
<th> </th><th>119</th><td class="r"><span> :trac:`<ins>12879</ins>`.</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>120</th><th>120</th><td class="l"><span></span></td>
</tr><tr>
<th>121</th><th>121</th><td class="l"><span> TESTS::</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
since the tobechanged file has <code>:trac:'???'</code>, hence, no backticks around ???.
</p>
</blockquote>
<p>
Did you include your trac_12876reviewer.patch (which fixes those backticks) in between trac_12876_categoryfix_abstract_classntrel11521.patch and trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch?
</p>
TicketnthieryWed, 02 May 2012 13:42:38 GMT
https://trac.sagemath.org/ticket/12876#comment:37
https://trac.sagemath.org/ticket/12876#comment:37
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:35" title="Comment 35">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I get four errors with <code>make ptestlong</code>:
...
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/categories/map.pyx"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 382:
sage: phi.category()
Expected:
Category of hom sets in Category of rings
Got:
Category of hom sets in Category of sets
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/categories/map.pyx", line 388:
sage: f.category()
Expected:
Join of Category of hom sets in Category of rings and Category of hom sets in Category of modules over Rational Field
Got:
Category of hom sets in Category of modules over Rational Field
**********************************************************************
</pre></blockquote>
<p>
This looks like the errors I was getting when I had removed completely
Rings.<a class="missing wiki">HomCategory?</a>. Can you double check that you have the latest
version of the reviewer patch from trac and that categories/rings.py
indeed has an (empty) class Rings.Homcategory?
</p>
TicketSimonKingWed, 02 May 2012 14:05:07 GMT
https://trac.sagemath.org/ticket/12876#comment:38
https://trac.sagemath.org/ticket/12876#comment:38
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:36" title="Comment 36">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Did you include your trac_12876reviewer.patch (which fixes those backticks) in between trac_12876_categoryfix_abstract_classntrel11521.patch and trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch?
</p>
</blockquote>
<p>
Nope! I thought that your review patch replaces mine.
</p>
TicketSimonKingWed, 02 May 2012 14:06:55 GMT
https://trac.sagemath.org/ticket/12876#comment:39
https://trac.sagemath.org/ticket/12876#comment:39
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:37" title="Comment 37">nthiery</a>:
</p>
<blockquote class="citation">
<p>
This looks like the errors I was getting when I had removed completely
Rings.<a class="missing wiki">HomCategory?</a>. Can you double check that you have the latest
version of the reviewer patch from trac and that categories/rings.py
indeed has an (empty) class Rings.Homcategory?
</p>
</blockquote>
<p>
No, categories/rings.py has no Homcategory. So, am I using the wrong patch?
</p>
TicketSimonKingWed, 02 May 2012 14:12:25 GMTwork_issues, description changed
https://trac.sagemath.org/ticket/12876#comment:40
https://trac.sagemath.org/ticket/12876#comment:40
<ul>
<li><strong>work_issues</strong>
changed from <em>Fix the reviewnt patch, fix doctests in map.pyx and homset.py</em> to <em>Fx doctests in map.pyx and homset.py</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=40">diff</a>)
</li>
</ul>
<p>
Application works when <em>both</em> reviewer patches are applied. I am changing the ticket description accordingly.
</p>
TicketSimonKingWed, 02 May 2012 14:14:07 GMT
https://trac.sagemath.org/ticket/12876#comment:41
https://trac.sagemath.org/ticket/12876#comment:41
<p>
PS: With
</p>
<pre class="wiki">king@mpc622:/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage$ hg qa
trac_12808classcall_speedupfh.patch
trac_12808_nested_class_cython.patch
trac_12808classcall_cdef.patch
trac12215_weak_cached_function.patch
trac12215_segfault_fixes.patch
trac_715_combined.patch
trac_11521_homset_weakcache_combined.patch
trac_12875categoryfix_abvar_homspacent.patch
trac_12877categoryfor_more_rings_and_schemesnt.patch
trac_12876_categoryfix_abstract_classntrel11521.patch
trac_12876reviewer.patch
trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch
</pre><p>
<code>sage.categories.rings.Rings.HomCategory</code> exists and is empty, as it is supposed to be, I guess.
</p>
TicketSimonKingWed, 02 May 2012 14:18:16 GMT
https://trac.sagemath.org/ticket/12876#comment:42
https://trac.sagemath.org/ticket/12876#comment:42
<p>
PS: We really should do something about source code inspection! See what happens if you do
</p>
<pre class="wiki">sage: sage.categories.rings.Rings.HomCategory??
</pre><p>
or
</p>
<pre class="wiki">sage: edit(sage.categories.rings.Rings.HomCategory,'vim')
</pre><p>
I thought I had fixed that in some ticket!!
</p>
TicketSimonKingWed, 02 May 2012 14:24:17 GMT
https://trac.sagemath.org/ticket/12876#comment:43
https://trac.sagemath.org/ticket/12876#comment:43
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:42" title="Comment 42">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I thought I had fixed that in some ticket!!
</p>
</blockquote>
<p>
I did: <a class="closed ticket" href="https://trac.sagemath.org/ticket/11768" title="enhancement: Get source code for parent/element classes of categories (closed: fixed)">#11768</a>, needing review (hinthint).
</p>
TicketnthieryWed, 02 May 2012 14:29:27 GMT
https://trac.sagemath.org/ticket/12876#comment:44
https://trac.sagemath.org/ticket/12876#comment:44
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:41" title="Comment 41">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
PS: With
</p>
<pre class="wiki">king@mpc622:/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage$ hg qa
trac_12808classcall_speedupfh.patch
trac_12808_nested_class_cython.patch
trac_12808classcall_cdef.patch
trac12215_weak_cached_function.patch
trac12215_segfault_fixes.patch
trac_715_combined.patch
trac_11521_homset_weakcache_combined.patch
trac_12875categoryfix_abvar_homspacent.patch
trac_12877categoryfor_more_rings_and_schemesnt.patch
trac_12876_categoryfix_abstract_classntrel11521.patch
trac_12876reviewer.patch
trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch
</pre><p>
<code>sage.categories.rings.Rings.HomCategory</code> exists and is empty, as it is supposed to be, I guess.
</p>
</blockquote>
<p>
Cool. I guess you are running the tests now, right?
</p>
TicketSimonKingWed, 02 May 2012 17:27:03 GMT
https://trac.sagemath.org/ticket/12876#comment:45
https://trac.sagemath.org/ticket/12876#comment:45
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:44" title="Comment 44">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Cool. I guess you are running the tests now, right?
</p>
</blockquote>
<p>
I have already verified that the two problematic tests pass. If it works, it will be a positive review.
</p>
<p>
Admittedly, I am running the test suite with <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> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/11768" title="enhancement: Get source code for parent/element classes of categories (closed: fixed)">#11768</a> applied as well...
</p>
TicketSimonKingWed, 02 May 2012 18:40:01 GMTwork_issues changed
https://trac.sagemath.org/ticket/12876#comment:46
https://trac.sagemath.org/ticket/12876#comment:46
<ul>
<li><strong>work_issues</strong>
changed from <em>Fx doctests in map.pyx and homset.py</em> to <em>Fx doctests in sageinspect.py</em>
</li>
</ul>
<p>
I'm afraid it still needs a tiny bit of work. <code>make ptestlong</code> resulted in (only) one error, namely
</p>
<pre class="wiki">sage t long force_lib "devel/sage/sage/misc/sageinspect.py"
**********************************************************************
File "/mnt/local/king/SAGE/stable/sage5.0.beta13/devel/sage/sage/misc/sageinspect.py", line 1495:
sage: sage_getsourcelines(HC)
Expected:
([' class HomCategory(HomCategory):\n',
' def extra_super_categories(self):\n',
...
' return (self.domain(), self.codomain(), self.category())\n'], ...)
Got:
([' class HomCategory(HomCategory):\n', ' pass\n'], 564)
**********************************************************************
1 items had failures:
1 of 24 in __main__.example_27
***Test Failed*** 1 failures.
For whitespace errors, see the file /mnt/local/king/.sage/tmp/sageinspect_13253.py
[14.0 s]

The following tests failed:
sage t long force_lib "devel/sage/sage/misc/sageinspect.py"
Total time for all tests: 14.0 seconds
</pre><p>
Apparently this is caused by emptying the HomCategory of Rings().
</p>
TicketSimonKingWed, 02 May 2012 18:40:27 GMT
https://trac.sagemath.org/ticket/12876#comment:47
https://trac.sagemath.org/ticket/12876#comment:47
<p>
PS: The obvious fix is to change the expected result in the test...
</p>
TicketSimonKingWed, 02 May 2012 18:49:13 GMT
https://trac.sagemath.org/ticket/12876#comment:48
https://trac.sagemath.org/ticket/12876#comment:48
<p>
Me stupid! The failing test is only introduced in <a class="closed ticket" href="https://trac.sagemath.org/ticket/11768" title="enhancement: Get source code for parent/element classes of categories (closed: fixed)">#11768</a>. So, I had better not apply more than the necessary patches...
</p>
<p>
Running the failing tests without the additional patch works.
</p>
<p>
Hence, repeating the test suite now... Sorry!
</p>
TicketSimonKingWed, 02 May 2012 21:21:12 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:49
https://trac.sagemath.org/ticket/12876#comment:49
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
Now it works. It is thus a positive review.
</p>
TicketnthieryWed, 02 May 2012 21:27:05 GMT
https://trac.sagemath.org/ticket/12876#comment:50
https://trac.sagemath.org/ticket/12876#comment:50
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:49" title="Comment 49">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Now it works. It is thus a positive review.
</p>
</blockquote>
<p>
Thanks! I can't wait for 5.1 :)
</p>
TicketSimonKingThu, 03 May 2012 12:26:12 GMTwork_issues deleted
https://trac.sagemath.org/ticket/12876#comment:51
https://trac.sagemath.org/ticket/12876#comment:51
<ul>
<li><strong>work_issues</strong>
<em>Fx doctests in sageinspect.py</em> deleted
</li>
</ul>
<p>
I forgot to remove the "work issues".
</p>
TicketjdemeyerMon, 07 May 2012 12:03:44 GMTmilestone changed
https://trac.sagemath.org/ticket/12876#comment:52
https://trac.sagemath.org/ticket/12876#comment:52
<ul>
<li><strong>milestone</strong>
changed from <em>sage5.1</em> to <em>sagepending</em>
</li>
</ul>
TicketjdemeyerMon, 07 May 2012 12:04:29 GMTwork_issues set
https://trac.sagemath.org/ticket/12876#comment:53
https://trac.sagemath.org/ticket/12876#comment:53
<ul>
<li><strong>work_issues</strong>
set to <em>commit message</em>
</li>
</ul>
<p>
The third patch needs a proper commit message.
</p>
TicketjdemeyerMon, 27 Aug 2012 11:34:30 GMTstatus, milestone changed
https://trac.sagemath.org/ticket/12876#comment:54
https://trac.sagemath.org/ticket/12876#comment:54
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>milestone</strong>
changed from <em>sagepending</em> to <em>sage5.4</em>
</li>
</ul>
TicketjdemeyerMon, 27 Aug 2012 11:42:27 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:55
https://trac.sagemath.org/ticket/12876#comment:55
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12875, #12877, #12215</em> to <em>#715, #11521, #12215</em>
</li>
</ul>
<p>
This needs to be rebased to sage5.3.rc0 + <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> + <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> + <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</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>.
</p>
TicketSimonKingTue, 28 Aug 2012 10:27:25 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_categoryfix_abstract_classskrel11521.patch</em>
</li>
</ul>
<p>
Replacement of <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a> wrt to the new patch versions at <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</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> etc.
</p>
TicketSimonKingTue, 28 Aug 2012 10:31:04 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:56
https://trac.sagemath.org/ticket/12876#comment:56
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=56">diff</a>)
</li>
</ul>
<p>
I have replaced Nicolas' first patch, under a new name. The changes became necessary by changes in the patches at <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> and <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>. Now I am running doctests.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classskrel11521.patch trac_12876reviewer.patch trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch
</p>
TicketSimonKingTue, 28 Aug 2012 11:15:20 GMTstatus, work_issues changed
https://trac.sagemath.org/ticket/12876#comment:57
https://trac.sagemath.org/ticket/12876#comment:57
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
changed from <em>commit message</em> to <em>fix doctests and segfaults</em>
</li>
</ul>
<p>
With sage5.3.b2 plus <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12215" title="defect: Memleak in UniqueRepresentation, @cached_method (closed: fixed)">#12215</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/13370" title="defect: Do not cache the result of is_Field externally (closed: fixed)">#13370</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/13378" title="defect: Do not cache the nonexistence of coerce/convert map too often, and do ... (closed: fixed)">#13378</a> and the patches from here, I get
</p>
<pre class="wiki">make ptest
...
The following tests failed:
sage t force_lib devel/sage/sage/interfaces/r.py # 1 doctests failed
sage t force_lib devel/sage/sage/rings/polynomial/infinite_polynomial_ring.py # 0 doctests failed
sage t force_lib devel/sage/sage/categories/hecke_modules.py # 1 doctests failed
</pre><p>
The error in hecke_modules.py is due to
</p>
<pre class="wiki">NotImplementedError: please implement _an_element_ for Set of Morphisms from Modular Forms space of dimension 3 for Congruence Subgroup Gamma0(7) of weight 4 over Rational Field to Modular Forms space of dimension 3 for Congruence Subgroup Gamma0(7) of weight 4 over Rational Field in Category of Hecke modules over Rational Field
</pre><p>
The error in expect/r.py looks strange to me:
</p>
<pre class="wiki">sage t force_lib devel/sage/sage/interfaces/r.py
**********************************************************************
File "/mnt/local/king/SAGE/prereleases/sage5.3.beta2/devel/sagemain/sage/interfaces/r.py", line 1169:
sage: os.path.realpath(tmpdir) == sageobj(r.getwd())
Expected:
True
Got:
False
**********************************************************************
1 items had failures:
1 of 7 in __main__.example_43
</pre><p>
The error is not reproducible, at least not if I run the test individually.
</p>
<p>
In infinite_polynomial_ring.py one gets a segfault. One does <em>not</em> get a segfault when running the tests with <code>verbose</code>. But gdb yields the following backtrace:
</p>
<pre class="wiki">#0 PyObject_Malloc (nbytes=123) at Objects/obmalloc.c:788
#1 0x00007ffff7a9bd5b in string_concat (a=0x7ffff7ed6730, bb=0x54c6180) at Objects/stringobject.c:1056
#2 0x00007ffff7a9dda5 in PyString_Concat (pv=0x7fffffffac58, w=<value optimized out>) at Objects/stringobject.c:3862
#3 0x00007ffff7a46025 in string_concatenate (v=0x7ffff7ed6730, w=0x54c6180, f=<value optimized out>, next_instr=<value optimized out>) at Python/ceval.c:4856
#4 0x00007ffff7af56c7 in PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:1548
#5 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=2, kws=0x49a5b70, kwcount=0,
defs=0x0, defcount=0, closure=0x0) at Python/ceval.c:3253
#6 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#7 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#8 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#9 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=2, kws=0x49a5930, kwcount=0,
defs=0x7ffff7f70968, defcount=1, closure=0x0) at Python/ceval.c:3253
#10 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#11 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#12 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#13 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=2, kws=0x50ed188, kwcount=0,
defs=0x7ffff7f708e8, defcount=1, closure=0x0) at Python/ceval.c:3253
#14 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#15 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#16 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#17 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=3, kws=0x50ecf90, kwcount=0,
defs=0x4918628, defcount=1, closure=0x0) at Python/ceval.c:3253
#18 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#19 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#20 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#21 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=3, kws=0x50ecd88, kwcount=0,
defs=0x7ffff7f708a8, defcount=1, closure=0x0) at Python/ceval.c:3253
#22 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#23 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#24 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#25 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=3, kws=0x50ec980, kwcount=0,
defs=0x7ffff7f5aa40, defcount=2, closure=0x0) at Python/ceval.c:3253
#26 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#27 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#28 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#29 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=3, kws=0x50eac40, kwcount=1,
defs=0x7ffff7ee9218, defcount=2, closure=0x0) at Python/ceval.c:3253
#30 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#31 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#32 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#33 0x00007ffff7af627b in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4107
#34 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#35 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#36 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=4, kws=0x0, kwcount=0,
defs=0x0, defcount=0, closure=0x0) at Python/ceval.c:3253
#37 0x00007ffff7a7a02c in function_call (func=0x491cb18, arg=0x476c050, kw=0x0) at Objects/funcobject.c:526
#38 0x00007ffff7a522c3 in PyObject_Call (func=0x491cb18, arg=<value optimized out>, kw=<value optimized out>) at Objects/abstract.c:2529
#39 0x00007ffff7a5fa0f in instancemethod_call (func=0x491cb18, arg=0x476c050, kw=0x0) at Objects/classobject.c:2578
#40 0x00007ffff7a522c3 in PyObject_Call (func=0x42995f0, arg=<value optimized out>, kw=<value optimized out>) at Objects/abstract.c:2529
#41 0x00007ffff7af40fd in do_call (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4239
#42 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4044
#43 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#44 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=4, kws=0x4d12cf0, kwcount=0,
defs=0x0, defcount=0, closure=0x0) at Python/ceval.c:3253
#45 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#46 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#47 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#48 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=2, kws=0x488e5b0, kwcount=3,
defs=0x491d388, defcount=3, closure=0x0) at Python/ceval.c:3253
#49 0x00007ffff7a7a123 in function_call (func=0x491cc80, arg=0x4906cb0, kw=0x494f090) at Objects/funcobject.c:526
#50 0x00007ffff7a522c3 in PyObject_Call (func=0x491cc80, arg=<value optimized out>, kw=<value optimized out>) at Objects/abstract.c:2529
#51 0x00007ffff7a5fa0f in instancemethod_call (func=0x491cc80, arg=0x4906cb0, kw=0x494f090) at Objects/classobject.c:2578
#52 0x00007ffff7a522c3 in PyObject_Call (func=0x4856be0, arg=<value optimized out>, kw=<value optimized out>) at Objects/abstract.c:2529
#53 0x00007ffff7af40fd in do_call (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4239
#54 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4044
#55 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#56 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=2, kws=0x4068fd0, kwcount=0,
defs=0x48c48d8, defcount=3, closure=0x0) at Python/ceval.c:3253
#57 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#58 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#59 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#60 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=0, kws=0x49af830, kwcount=10,
defs=0x49104a8, defcount=10, closure=0x0) at Python/ceval.c:3253
#61 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#62 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#63 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#64 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=1, kws=0x6d8fb8, kwcount=3,
defs=0x4910310, defcount=10, closure=0x0) at Python/ceval.c:3253
#65 0x00007ffff7af5550 in fast_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4117
#66 call_function (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:4042
#67 PyEval_EvalFrameEx (f=<value optimized out>, throwflag=<value optimized out>) at Python/ceval.c:2666
#68 0x00007ffff7af7125 in PyEval_EvalCodeEx (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>, args=<value optimized out>, argcount=0, kws=0x0, kwcount=0,
defs=0x0, defcount=0, closure=0x0) at Python/ceval.c:3253
#69 0x00007ffff7af7262 in PyEval_EvalCode (co=<value optimized out>, globals=<value optimized out>, locals=<value optimized out>) at Python/ceval.c:667
#70 0x00007ffff7b19760 in run_mod (fp=0x6c66c0, filename=<value optimized out>, start=<value optimized out>, globals=0x640280, locals=0x640280, closeit=1, flags=0x7fffffffd380)
at Python/pythonrun.c:1353
#71 PyRun_FileExFlags (fp=0x6c66c0, filename=<value optimized out>, start=<value optimized out>, globals=0x640280, locals=0x640280, closeit=1, flags=0x7fffffffd380) at Python/pythonrun.c:1339
#72 0x00007ffff7b1a1ff in PyRun_SimpleFileExFlags (fp=0x6c66c0, filename=0x7fffffffd8ff "/mnt/local/king/.sage/tmp/infinite_polynomial_ring_15153.py", closeit=1, flags=0x7fffffffd380)
at Python/pythonrun.c:943
#73 0x00007ffff7b2d4b5 in Py_Main (argc=<value optimized out>, argv=<value optimized out>) at Modules/main.c:639
#74 0x00007ffff6e1ec8d in __libc_start_main (main=<value optimized out>, argc=<value optimized out>, ubp_av=<value optimized out>, init=<value optimized out>, fini=<value optimized out>,
rtld_fini=<value optimized out>, stack_end=0x7fffffffd498) at libcstart.c:228
#75 0x0000000000400619 in _start ()
</pre><p>
Could very well be that this is (once again) due to the new weak caches. But I will also rerun without <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> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/13378" title="defect: Do not cache the nonexistence of coerce/convert map too often, and do ... (closed: fixed)">#13378</a>.
</p>
TicketjdemeyerTue, 28 Aug 2012 11:31:01 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:58
https://trac.sagemath.org/ticket/12876#comment:58
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215</em> to <em>#715, #11521, #12215, #12313</em>
</li>
</ul>
TicketSimonKingTue, 28 Aug 2012 14:40:22 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:59
https://trac.sagemath.org/ticket/12876#comment:59
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=59">diff</a>)
</li>
</ul>
<p>
Got it!
</p>
<p>
Infinite polynomial rings were using <code>WeakKeyDict</code> in <code>_has_coerce_map_from_</code>for caching whether there is a coercion. That has probably been a bad idea, because the methods that are really used (<code>has_coerce_map</code>) have a cache anyway.
</p>
<p>
Removing the additional cache fixes the segfault.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classskrel11521.patch trac_12876reviewer.patch trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch trac_12876_fix_infinite_polynomial_ring.patch
</p>
TicketSimonKingTue, 28 Aug 2012 15:33:21 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:60
https://trac.sagemath.org/ticket/12876#comment:60
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215, #12313</em> to <em>#11521, #12215, #12313</em>
</li>
</ul>
<p>
One has to remove the dependency <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>, because <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> are mutually dependent, but <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> needs to be applied first.
</p>
<p>
Anyway. There just remains the error in sage t force_lib devel/sage/sage/categories/hecke_modules.py, which amounts to writing a _an_element_ method. The error with r seems to have been noise.
</p>
TicketSimonKingTue, 28 Aug 2012 15:56:39 GMT
https://trac.sagemath.org/ticket/12876#comment:61
https://trac.sagemath.org/ticket/12876#comment:61
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:60" title="Comment 60">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Anyway. There just remains the error in sage t force_lib devel/sage/sage/categories/hecke_modules.py, which amounts to writing a _an_element_ method.
</p>
</blockquote>
<p>
... or: One could replace that test  after all, the test is new (introduced in the first patch).
</p>
TicketSimonKingTue, 28 Aug 2012 17:08:55 GMT
https://trac.sagemath.org/ticket/12876#comment:62
https://trac.sagemath.org/ticket/12876#comment:62
<p>
I think implementing a <code>_an_element_</code> is easy enough that I can do it in another reviewer patch. One thing doesn't work as described in the documentation, though. It says that a homomorphism of Hecke algebras can not only be obtained from a matrix (that works) but from an element of a Hecke algebra (I suppose: An element of the codomain). However, this isn't implemented.
</p>
<p>
I am not competent to implement it. So, I suggest that I start with implementing <code>_an_element_</code>, try to use the category framework on these homsets (i.e., define the Element attribute), and very important: Make the homsets unique upon pickling. Namely:
</p>
<pre class="wiki">sage: M = ModularForms(Gamma0(7), 4)
sage: H = Hom(M,M)
sage: H is loads(dumps(H))
False
</pre><p>
so that by consequence several tests from the test suite fail.
</p>
TicketSimonKingTue, 28 Aug 2012 17:18:58 GMT
https://trac.sagemath.org/ticket/12876#comment:63
https://trac.sagemath.org/ticket/12876#comment:63
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:62" title="Comment 62">SimonKing</a>:
</p>
<blockquote class="citation">
<pre class="wiki">sage: M = ModularForms(Gamma0(7), 4)
sage: H = Hom(M,M)
sage: H is loads(dumps(H))
False
</pre></blockquote>
<p>
... which has two reasons: <code>H</code> has no <code>__reduce__</code> method, and <code>M</code> has no reduce method.
</p>
TicketnthieryTue, 28 Aug 2012 20:22:27 GMT
https://trac.sagemath.org/ticket/12876#comment:64
https://trac.sagemath.org/ticket/12876#comment:64
<blockquote>
<p>
Hi Simon!
</p>
</blockquote>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:62" title="Comment 62">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I think implementing a <code>_an_element_</code> is easy enough that I can do it in another reviewer patch. One thing doesn't work as described in the documentation, though. It says that a homomorphism of Hecke algebras can not only be obtained from a matrix (that works) but from an element of a Hecke algebra (I suppose: An element of the codomain). However, this isn't implemented.
</p>
<p>
I am not competent to implement it. So, I suggest that I start with implementing <code>_an_element_</code>, try to use the category framework on these homsets (i.e., define the Element attribute), and very important: Make the homsets unique upon pickling. Namely:
</p>
<pre class="wiki">sage: M = ModularForms(Gamma0(7), 4)
sage: H = Hom(M,M)
sage: H is loads(dumps(H))
False
</pre><p>
so that by consequence several tests from the test suite fail.
</p>
</blockquote>
<p>
Feel free to just skip the failing tests and mention them, say on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12879" title="defect: TestSuite failures for HeckeModule Homsets (closed: fixed)">#12879</a>.
</p>
<p>
And thanks a lot for all the work you put in rebasing/finalizing this patch! I'll try to look at it tomorrow.
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingTue, 28 Aug 2012 21:13:59 GMT
https://trac.sagemath.org/ticket/12876#comment:65
https://trac.sagemath.org/ticket/12876#comment:65
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:64" title="Comment 64">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Feel free to just skip the failing tests and mention them, say on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12879" title="defect: TestSuite failures for HeckeModule Homsets (closed: fixed)">#12879</a>.
</p>
</blockquote>
<p>
... and to remove the erroneous documentation (or perhaps: Marking it as "TODO").
</p>
<p>
I was confident that I have produced a patch (not posted, though) that does the right thing with the categories of Hecke homsets. In fact, the full <code>TestSuite</code> of these homsets passes. However, a surprising amount of tests fail, all over sage. So, I guess you are right: One should work around for now, and fix it for real in <a class="closed ticket" href="https://trac.sagemath.org/ticket/12879" title="defect: TestSuite failures for HeckeModule Homsets (closed: fixed)">#12879</a>.
</p>
<p>
Cheers,
Simon
</p>
TicketSimonKingWed, 29 Aug 2012 09:07:06 GMT
https://trac.sagemath.org/ticket/12876#comment:66
https://trac.sagemath.org/ticket/12876#comment:66
<p>
I have updated the last patch, so that more tests from the failing <code>TestSuite</code> are skipped. Let's see if the patchbot is happy.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classskrel11521.patch trac_12876reviewer.patch trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch trac_12876_fix_infinite_polynomial_ring.patch
</p>
TicketSimonKingWed, 29 Aug 2012 13:00:34 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_fix_infinite_polynomial_ring.patch</em>
</li>
</ul>
<p>
Do not weakcache things in infinite_polynomial_ring that are cached anyway. Skip more tests in Hecke homset's <code>TestSuite</code>
</p>
TicketSimonKingWed, 29 Aug 2012 13:05:01 GMT
https://trac.sagemath.org/ticket/12876#comment:67
https://trac.sagemath.org/ticket/12876#comment:67
<p>
Oops, I thought that I had an outdated version of my last patch, but apparently it has been the correct one. Anyway, I'll restart the patchbot and will run the tests on my own machine as well, and will of course have a look at Nicolas' patches
</p>
<p>
From Nicolas' comment on <a class="closed ticket" href="https://trac.sagemath.org/ticket/12879" title="defect: TestSuite failures for HeckeModule Homsets (closed: fixed)">#12879</a>, I understood that he accepts to make the <code>TestSuite</code> temporarily silent and leave a proper fix to the number theorists. I guess that's a healthy attitude...
</p>
<p>
I hope that Nicolas also likes the fix for infinite polynomial rings.
</p>
<p>
Apply trac_12876_categoryfix_abstract_classskrel11521.patch trac_12876reviewer.patch trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch trac_12876_fix_infinite_polynomial_ring.patch
</p>
TicketSimonKingWed, 29 Aug 2012 13:37:30 GMT
https://trac.sagemath.org/ticket/12876#comment:68
https://trac.sagemath.org/ticket/12876#comment:68
<p>
Some questions, Nicolas:
</p>
<p>
The first patch contains
</p>
<pre class="wiki">sage: category = FiniteDimensionalModulesWithBasis(QQ)
sage: X = CombinatorialFreeModule(QQ, [1,2,3], category = category); X.rename("X")
sage: Y = CombinatorialFreeModule(QQ, [1,2,3,4], category = category); Y.rename("Y")
sage: H = Hom(X, Y)
sage: H.zero().category_for()
Category of finite dimensional modules with basis over Rational Field
</pre><p>
Why are X and Y renamed? There is yet another location where things are renamed, namely
</p>
<pre class="wiki">Diagonal functions can be constructed using the ``diagonal`` option::
sage: X = CombinatorialFreeModule(QQ, [1,2,3,4]); X.rename("X")
sage: Y = CombinatorialFreeModule(QQ, [1,2,3,4], key="Y"); Y.rename("Y")
sage: H = Hom(X, Y)
</pre><p>
Here, I also wonder about the "key" argument.
</p>
<p>
In sage.rings.ring.pyx, you state:
</p>
<pre class="wiki"> """
This temporary alias is here for those instances of :class:`Ring`
that are not yet properly in the :class:`Rings`() category.
TESTS::
sage: A = Ring(ZZ)
sage: A._Hom_.__module__
'sage.categories.rings'
sage: Hom(A,A).__class__
<class 'sage.rings.homset.RingHomset_generic_with_category'>
"""
_Hom_ = Rings.ParentMethods.__dict__['_Hom_']
</pre><p>
Are there any rings that do not initialise the category, after <a class="closed ticket" href="https://trac.sagemath.org/ticket/9138" title="defect: Categories for all rings (closed: fixed)">#9138</a>? Hence, what would happen without that alias? Perhaps I'll test it, after finishing the tests for the original patches.
</p>
TicketSimonKingWed, 29 Aug 2012 16:21:16 GMT
https://trac.sagemath.org/ticket/12876#comment:69
https://trac.sagemath.org/ticket/12876#comment:69
<p>
All tests passed, and the patches look good to me, modulo the questions that I asked above. I will now rerun the tests without the alias for _Hom_.
</p>
TicketSimonKingWed, 29 Aug 2012 17:08:26 GMT
https://trac.sagemath.org/ticket/12876#comment:70
https://trac.sagemath.org/ticket/12876#comment:70
<p>
When removing the _Hom_ alias, I get
</p>
<pre class="wiki"> sage t force_lib devel/sage/sage/rings/multi_power_series_ring_element.py # 5 doctests failed
</pre><p>
So, apparently it <em>is</em> needed.
</p>
TicketSimonKingWed, 29 Aug 2012 21:16:59 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_another_reviewer.patch</em>
</li>
</ul>
<p>
Another reviewer patch
</p>
TicketSimonKingWed, 29 Aug 2012 21:24:16 GMTwork_issues, description changed
https://trac.sagemath.org/ticket/12876#comment:71
https://trac.sagemath.org/ticket/12876#comment:71
<ul>
<li><strong>work_issues</strong>
changed from <em>fix doctests and segfaults</em> to <em>add commit message to one patch</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=71">diff</a>)
</li>
</ul>
<p>
I give it a positive review, modulo commit message of <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a> and modulo the small reviewer patch that I have attached. I think it should be <code>.. todo::</code> and not <code>..TODO::</code> (note the blank space).
</p>
<p>
So, when you added a commit message to your patch, then please change it into a positive review!
</p>
<p>
Apply trac_12876_categoryfix_abstract_classskrel11521.patch trac_12876reviewer.patch trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch trac_12876_fix_infinite_polynomial_ring.patch trac_12876_another_reviewer.patch
</p>
TicketSimonKingThu, 30 Aug 2012 08:06:51 GMT
https://trac.sagemath.org/ticket/12876#comment:72
https://trac.sagemath.org/ticket/12876#comment:72
<p>
Here is the problem with removing the nasty alias:
</p>
<pre class="wiki">sage: A.<a,b> = PowerSeriesRing(QQ)
sage: A._is_category_initialized()
False
sage: isinstance(A,Ring)
True
</pre><p>
Hence, apparently <code>Ring.__init__</code> is not called on <code>PowerSeriesRing</code>. That's nasty, but I think in order to make any progress it would be better to leave that to a different ticket.
</p>
TicketSimonKingThu, 30 Aug 2012 08:10:09 GMT
https://trac.sagemath.org/ticket/12876#comment:73
https://trac.sagemath.org/ticket/12876#comment:73
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:72" title="Comment 72">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Hence, apparently <code>Ring.__init__</code> is not called on <code>PowerSeriesRing</code>. That's nasty, but I think in order to make any progress it would be better to leave that to a different ticket.
</p>
</blockquote>
<p>
See <a class="closed ticket" href="https://trac.sagemath.org/ticket/13412" title="defect: PowerSeriesRing should call Ring.__init__ (closed: fixed)">#13412</a>
</p>
TicketnthieryThu, 30 Aug 2012 08:47:31 GMT
https://trac.sagemath.org/ticket/12876#comment:74
https://trac.sagemath.org/ticket/12876#comment:74
<blockquote>
<p>
Hi Simon,
</p>
</blockquote>
<p>
Sorry, I am slow answering; I was immersed all afternoon in tracking
down a nasty bug where <a class="missing wiki">UniqueRepresentation?</a> was violated. It turned
out that the <a class="missing wiki">UniqueRepresentation?</a> dict had to equal entries in its
keys (gasp), and that was due to a tricky mro order for a non trivial
inheritance involving Category_singleton and <a class="missing wiki">FastHash?</a> which got the
hash to change over time ... Anyway ...
</p>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:68" title="Comment 68">SimonKing</a>:
</p>
<blockquote class="citation">
<pre class="wiki">Diagonal functions can be constructed using the ``diagonal`` option::
sage: X = CombinatorialFreeModule(QQ, [1,2,3,4]); X.rename("X")
sage: Y = CombinatorialFreeModule(QQ, [1,2,3,4], key="Y"); Y.rename("Y")
sage: H = Hom(X, Y)
</pre><p>
Why are X and Y renamed? There is yet another location where things are renamed, namely
Here, I also wonder about the "key" argument.
</p>
</blockquote>
<p>
I do this fairly systematically in the tests in modules_with_basis. It
makes it immediate to check that the domain and codomain are set
properly when looking at the repr of a morphism or homset. They key is
to force the domain and codomain to be distinct (otherwise, and this
happened to me, if the code accidently interchanges the two it will
get unnoticed).
</p>
<p>
Well, and for good or bad, once you rename a given parent in a
doctest, you have to rename consistently this parent in all other
doctests in the same file. Or to call X.rename() at the end of the
doctest.
</p>
TicketnthieryThu, 30 Aug 2012 08:49:14 GMT
https://trac.sagemath.org/ticket/12876#comment:75
https://trac.sagemath.org/ticket/12876#comment:75
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:72" title="Comment 72">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Hence, apparently <code>Ring.__init__</code> is not called on <code>PowerSeriesRing</code>. That's nasty, but I think in order to make any progress it would be better to leave that to a different ticket.
</p>
</blockquote>
<p>
+1
</p>
<p>
Can you just add a mini comment on <a class="closed ticket" href="https://trac.sagemath.org/ticket/13412" title="defect: PowerSeriesRing should call Ring.__init__ (closed: fixed)">#13412</a> stating that once it's done the alias can be removed?
</p>
<p>
Thanks!
</p>
TicketnthieryThu, 30 Aug 2012 09:05:53 GMT
https://trac.sagemath.org/ticket/12876#comment:76
https://trac.sagemath.org/ticket/12876#comment:76
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:71" title="Comment 71">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I give it a positive review, modulo commit message of
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a>
and modulo the small reviewer patch that I have attached. I think it
should be <code>.. todo::</code> and not <code>..TODO::</code> (note the blank space).
</p>
</blockquote>
<p>
Thanks for fixing the space. Apparently, there is some sort of
consensus for using .. TODO:: rather than .. todo:: (same thing
for seealso, ...).
</p>
<p>
I have checked the change on infinite polynomial rings, and am ok with
it.
</p>
<p>
Do you mind folding all patches together? Now that we have checked the
little steps, that will leave a better overview for the future. And
that will take care of the commit messages :)
</p>
<p>
(or I can do it, but I need to get the latest beta version first).
</p>
<p>
And then, it's ready for positive review! Yippee!
</p>
<p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketSimonKingThu, 30 Aug 2012 10:22:17 GMT
https://trac.sagemath.org/ticket/12876#comment:77
https://trac.sagemath.org/ticket/12876#comment:77
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:76" title="Comment 76">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:71" title="Comment 71">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I give it a positive review, modulo commit message of
<a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Attachment 'trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch' in Ticket #12876">trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch" title="Download"></a>
and modulo the small reviewer patch that I have attached. I think it
should be <code>.. todo::</code> and not <code>..TODO::</code> (note the blank space).
</p>
</blockquote>
<p>
Thanks for fixing the space. Apparently, there is some sort of
consensus for using .. TODO:: rather than .. todo:: (same thing
for seealso, ...).
</p>
</blockquote>
<p>
I wouldn't mind to have it upper case.
</p>
<blockquote class="citation">
<p>
(or I can do it, but I need to get the latest beta version first).
</p>
</blockquote>
<p>
I don't think you need any beta version if you want to add a commit message. I think editing the patch and replacing the line <code>[mq]: trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</code> by, say <code>#12876: Use generic code for sage.categories.rings.Rings.HomCategory</code> would be enough.
</p>
TicketnthieryThu, 30 Aug 2012 10:52:42 GMT
https://trac.sagemath.org/ticket/12876#comment:78
https://trac.sagemath.org/ticket/12876#comment:78
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:77" title="Comment 77">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I don't think you need any beta version if you want to add a commit message. I think editing the patch and replacing the line <code>[mq]: trac_12876_categoryfix_abstract_classntrel11521reviewnt.patch</code> by, say <code>#12876: Use generic code for sage.categories.rings.Rings.HomCategory</code> would be enough.
</p>
</blockquote>
<p>
I meant for folding the patches together!
</p>
TicketSimonKingThu, 30 Aug 2012 11:13:55 GMT
https://trac.sagemath.org/ticket/12876#comment:79
https://trac.sagemath.org/ticket/12876#comment:79
<p>
Why folding? Perhaps that's a question to the release manager. But are four patches substantially more difficult to merge than one patch?
</p>
TicketnthieryThu, 30 Aug 2012 12:14:26 GMT
https://trac.sagemath.org/ticket/12876#comment:80
https://trac.sagemath.org/ticket/12876#comment:80
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:79" title="Comment 79">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Why folding? Perhaps that's a question to the release manager. But are four patches substantially more difficult to merge than one patch?
</p>
</blockquote>
<p>
It's not so much about the release manager than for whoever will come
back to this ticket in the future, and will want to have a synthetic
view of what the patch(es) does.
</p>
<p>
Separate patches are temporarily good for incremental review. They can
be interesting too in the long run when they do independent
changes. But when they pile on top of each other (typically for
trivial typo fixes) it goes in the way of the reader. Typically (s)he
will read the main patch, see a typo, wonder if it has been fixed,
will have to dig through the followup patches, think about the order
in which they are to be applied, etc.
</p>
TicketSimonKingThu, 30 Aug 2012 13:24:18 GMTdependencies, description changed
https://trac.sagemath.org/ticket/12876#comment:81
https://trac.sagemath.org/ticket/12876#comment:81
<ul>
<li><strong>dependencies</strong>
changed from <em>#11521, #12215, #12313</em> to <em>#11521, #12215, #12313, #13412</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=81">diff</a>)
</li>
</ul>
<p>
I produced a combined patch. I added <a class="closed ticket" href="https://trac.sagemath.org/ticket/13412" title="defect: PowerSeriesRing should call Ring.__init__ (closed: fixed)">#13412</a>, because it allows to remove the ugly alias for <code>_Hom_</code> in sage.rings.ring. Because of that last change, I keep it "needs review" for now, but will change it into positive review provided the tests pass.
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
TicketnthieryThu, 30 Aug 2012 13:45:14 GMT
https://trac.sagemath.org/ticket/12876#comment:82
https://trac.sagemath.org/ticket/12876#comment:82
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:81" title="Comment 81">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
I produced a combined patch. I added <a class="closed ticket" href="https://trac.sagemath.org/ticket/13412" title="defect: PowerSeriesRing should call Ring.__init__ (closed: fixed)">#13412</a>, because it allows to remove the ugly alias for <code>_Hom_</code> in sage.rings.ring. Because of that last change, I keep it "needs review" for now, but will change it into positive review provided the tests pass.
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
</blockquote>
<p>
Excellent, thanks!
</p>
<p>
Is there any of the old patches that you would like to keep for posterity, or should I just wipe them up?
</p>
<p>
Cheers,
</p>
TicketSimonKingThu, 30 Aug 2012 13:55:05 GMT
https://trac.sagemath.org/ticket/12876#comment:83
https://trac.sagemath.org/ticket/12876#comment:83
<p>
I wouldn't remove the old patches from trac  and actually I wouldn't be able to remove them , unless there are problems with disc space etc. And IF there will eventually be problems with disc space, I guess a better solution would be to start deleting the attachments of tickets that are merged since, say, at least one year.
</p>
TicketSimonKingThu, 30 Aug 2012 14:30:08 GMTstatus, work_issues changed
https://trac.sagemath.org/ticket/12876#comment:84
https://trac.sagemath.org/ticket/12876#comment:84
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
changed from <em>add commit message to one patch</em> to <em>Understand why the crash in infinite_polynomial_ring is not fixed and what happens with r</em>
</li>
</ul>
<p>
That's bad. Pathbot reports
</p>
<pre class="wiki"> sage t force_lib devel/sage12876/sage/rings/polynomial/infinite_polynomial_ring.py # Killed/crashed
sage t force_lib devel/sage12876/sage/interfaces/r.py # 1 doctests failed
</pre><p>
The crash in infinite_polynomial_ring.py was supposed to be fixed with one of the folded patches, namely removing the additional weak cache. The second one has occurred on my computer as well  but was not reproducible, so I thought it was just some noise.
</p>
<p>
Bad.
</p>
TicketSimonKingThu, 30 Aug 2012 15:23:55 GMT
https://trac.sagemath.org/ticket/12876#comment:85
https://trac.sagemath.org/ticket/12876#comment:85
<p>
I tried to stresstest my machine, running 16 copies of the infinite_polynomial_ring tests in parallel on only 4 kernels, but I was not able to reproduce the segfault.
</p>
<p>
Concerning the other error, perhaps one can add the following patch:
</p>
<div class="wikicode"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>sage/interfaces/r.py</a>
</h2>
<pre>diff git a/sage/interfaces/r.py b/sage/interfaces/r.py</pre>
<table class="tracdiff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File a/sage/interfaces/r.py">
a
</th>
<th title="File b/sage/interfaces/r.py">
b
</th>
<td><em></em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>1166</th><th>1166</th><td class="l"><span></span></td>
</tr><tr>
<th>1167</th><th>1167</th><td class="l"><span> ::</span></td>
</tr><tr>
<th>1168</th><th>1168</th><td class="l"><span></span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>1169</th><th> </th><td class="l"><span> sage: os.path.realpath(tmpdir) == <del>sageobj(r.getwd(</del>))</span></td>
</tr>
<tr class="last">
<th> </th><th>1169</th><td class="r"><span> sage: os.path.realpath(tmpdir) == <ins>os.path.realpath(sageobj(r.getwd()</ins>))</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>1170</th><th>1170</th><td class="l"><span> True</span></td>
</tr><tr>
<th>1171</th><th>1171</th><td class="l"><span> """</span></td>
</tr><tr>
<th>1172</th><th>1172</th><td class="l"><span> self.execute('setwd(%r)' % dir)</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div>
TicketSimonKingThu, 30 Aug 2012 15:28:53 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:86
https://trac.sagemath.org/ticket/12876#comment:86
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=86">diff</a>)
</li>
</ul>
<p>
Let's try, even though I have no idea about the segfault:
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch trac_12876_r_test.patch
</p>
TicketSimonKingThu, 30 Aug 2012 15:56:48 GMT
https://trac.sagemath.org/ticket/12876#comment:87
https://trac.sagemath.org/ticket/12876#comment:87
<p>
OK, the rtest works now. But the segfault remains. I wish I could reproduce it, preferably in verbose mode.
</p>
TicketSimonKingThu, 30 Aug 2012 21:24:49 GMT
https://trac.sagemath.org/ticket/12876#comment:88
https://trac.sagemath.org/ticket/12876#comment:88
<p>
I tried to valgrind the failing segfault (Nils Bruin recommended valgrind and posted a sage.supp at <a class="closed ticket" href="https://trac.sagemath.org/ticket/11918" title="defect: Sage should ship its Valgrind suppressions file, or not insist on its ... (closed: fixed)">#11918</a>), in the hope that it'll find something fishy even if it doesn't result in a segfault.
</p>
<p>
When I run the tests of sage/rings/polynomial/infinite_polynomial_ring.py, I do not get a SIGILL. But I do get a considerable amount of lost memory:
</p>
<pre class="wiki">==13541== 13,936 bytes in 1 blocks are definitely lost in loss record 8,673 of 8,997
==13541== at 0x4C244E8: malloc (vg_replace_malloc.c:236)
==13541== by 0x21E8984F: omAllocFromSystem (omAllocSystem.c:184)
==13541== by 0x21E89A21: omAllocLarge (omAllocSystem.c:39)
==13541== by 0x21BB3A00: iiAllStart(procinfo*, char*, feBufferTypes, int) (omalloc.h:2432)
==13541== by 0x21BB3B95: iiPStart(idrec*, sleftv*) (iplib.cc:360)
==13541== by 0x21BB4148: iiMake_proc(idrec*, sip_package*, sleftv*) (iplib.cc:482)
==13541== by 0x2239B64D: __pyx_f_4sage_4libs_8singular_8function_call_function(__pyx_obj_4sage_4libs_8singular_8function_SingularFunction*, _object*, _object*, __pyx_opt_args_4sage_4libs_8singular_8f
unction_call_function*) (function.cpp:13241)
==13541== by 0x2239CBA8: __pyx_pw_4sage_4libs_8singular_8function_16SingularFunction_5__call__(_object*, _object*, _object*) (function.cpp:11924)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
==13541== by 0x4F160FC: PyEval_EvalFrameEx (ceval.c:4239)
==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253)
==13541== by 0x4E9C122: function_call (funcobject.c:526)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
==13541== by 0x4F14C59: PyEval_EvalFrameEx (ceval.c:4334)
==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253)
==13541== by 0x4E9C122: function_call (funcobject.c:526)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
==13541== by 0x4F14C59: PyEval_EvalFrameEx (ceval.c:4334)
==13541== by 0x4F19124: PyEval_EvalCodeEx (ceval.c:3253)
==13541== by 0x4E9C122: function_call (funcobject.c:526)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
==13541== by 0xB29B841: __pyx_pw_4sage_4misc_9cachefunc_12CachedMethod_3_instance_call (cachefunc.c:9733)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
==13541== by 0xB29C7D4: __pyx_pw_4sage_4misc_9cachefunc_18CachedMethodCaller_7__call__ (cachefunc.c:7254)
==13541== by 0x4E742C2: PyObject_Call (abstract.c:2529)
</pre><p>
Is there a way to find out what singular_function or what cached method are involved?
</p>
TicketSimonKingSat, 01 Sep 2012 23:10:11 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:89
https://trac.sagemath.org/ticket/12876#comment:89
<ul>
<li><strong>dependencies</strong>
changed from <em>#11521, #12215, #12313, #13412</em> to <em>#11521, #12215, #12313, #13412. #13145</em>
</li>
</ul>
<p>
It seems that <a class="closed ticket" href="https://trac.sagemath.org/ticket/13145" title="defect: Sage's noncommutative rings don't always increment a refcount (closed: fixed)">#13145</a> fixes the segfault that was sometimes observed at <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>. Perhaps it helps here as well? Let's try!
</p>
TicketSimonKingSun, 02 Sep 2012 07:09:25 GMT
https://trac.sagemath.org/ticket/12876#comment:90
https://trac.sagemath.org/ticket/12876#comment:90
<p>
Too bad, <a class="closed ticket" href="https://trac.sagemath.org/ticket/13145" title="defect: Sage's noncommutative rings don't always increment a refcount (closed: fixed)">#13145</a> didn't help.
</p>
<p>
I'd really need some help to trace the problem down. Could someone run the test in verbose mode on one of the machines exposing the segfault, so that one can see whether the problem is during the tests or while shutting down Sage? Can someone produce (and post) a backtrace?
</p>
TicketSimonKingMon, 03 Sep 2012 09:35:45 GMT
https://trac.sagemath.org/ticket/12876#comment:91
https://trac.sagemath.org/ticket/12876#comment:91
<p>
There is a potential name conflict between an attribute S of Action and <code>RingHomomorphism</code>. The former has been turned into a weak reference at <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a>. I am now trying to rename the attribute of Action, and updated the main patch at <a class="closed ticket" href="https://trac.sagemath.org/ticket/715" title="defect: Parents probably not reclaimed due to too much caching (closed: fixed)">#715</a> accordingly.
</p>
<p>
Hence, kicking the patchbot now. Hope it works!
</p>
TicketSimonKingMon, 03 Sep 2012 12:35:41 GMT
https://trac.sagemath.org/ticket/12876#comment:92
https://trac.sagemath.org/ticket/12876#comment:92
<p>
It doesn't work. Hence, again I'd like to finally see a backtrace of the segfault. So far, I tried to reproduce the segfault on my linux laptop, but to no avail. Next, I'll try bsd.math.
</p>
TicketSimonKingMon, 03 Sep 2012 14:36:29 GMT
https://trac.sagemath.org/ticket/12876#comment:93
https://trac.sagemath.org/ticket/12876#comment:93
<p>
Now I am totally puzzled. On bsd.math, admittedly with the new GAP 4.5.5 spkg, I get
</p>
<pre class="wiki">The following tests failed:
sage t force_lib devel/sage/sage/combinat/combinatorial_algebra.py # 4 doctests failed
sage t force_lib devel/sage/sage/combinat/partition.py # 3 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/classical.py # 11 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/dual.py # 89 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/elementary.py # 9 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/hall_littlewood.py # 61 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/homogeneous.py # 9 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/jack.py # 68 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/kschur.py # 15 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/llt.py # 50 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/monomial.py # 16 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/ns_macdonald.py # 2 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/macdonald.py # 106 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/orthotriang.py # 25 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/powersum.py # 17 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/schur.py # 13 doctests failed
sage t force_lib devel/sage/sage/combinat/sf/sfa.py # 291 doctests failed
sage t force_lib devel/sage/sage/combinat/species/composition_species.py # 4 doctests failed
sage t force_lib devel/sage/sage/combinat/species/functorial_composition_species.py # 3 doctests failed
sage t force_lib devel/sage/sage/combinat/species/generating_series.py # 38 doctests failed
sage t force_lib devel/sage/sage/combinat/species/library.py # 4 doctests failed
sage t force_lib devel/sage/sage/combinat/species/species.py # 4 doctests failed
</pre><p>
but no segfault. Does one need <a class="closed ticket" href="https://trac.sagemath.org/ticket/5457" title="enhancement: Refactor symmetric functions and kbounded subspace (closed: fixed)">#5457</a>?
</p>
TicketjdemeyerMon, 03 Sep 2012 14:40:26 GMT
https://trac.sagemath.org/ticket/12876#comment:94
https://trac.sagemath.org/ticket/12876#comment:94
<p>
Did you forget a <code>./sage b</code> by chance?
</p>
TicketSimonKingMon, 03 Sep 2012 14:48:06 GMT
https://trac.sagemath.org/ticket/12876#comment:95
https://trac.sagemath.org/ticket/12876#comment:95
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:94" title="Comment 94">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Did you forget a <code>./sage b</code> by chance?
</p>
</blockquote>
<p>
Doesn't <code>make ptest</code> start with <code>./sage b</code>?
</p>
TicketSimonKingMon, 03 Sep 2012 19:39:24 GMT
https://trac.sagemath.org/ticket/12876#comment:96
https://trac.sagemath.org/ticket/12876#comment:96
<p>
It is really really REALLY frustrating. Volker's patchbot keeps reporting a segfault, but I can not reproduce it on bsd.math, With <a class="closed ticket" href="https://trac.sagemath.org/ticket/5457" title="enhancement: Refactor symmetric functions and kbounded subspace (closed: fixed)">#5457</a> and the new GAP spkg, make ptest just reports a single error:
</p>
<pre class="wiki">sage t force_lib "devel/sage/sage/interfaces/gap.py"
**********************************************************************
File "/scratch/sking/sage5.3.rc1/devel/sage/sage/interfaces/gap.py", line 809:
sage: gap(2)
Expected:
2
Got:
<BLANKLINE>
**********************************************************************
1 items had failures:
1 of 4 in __main__.example_21
***Test Failed*** 1 failures.
For whitespace errors, see the file /Users/SimonKing/.sage//tmp/gap_70056.py
[29.6 s]

The following tests failed:
sage t force_lib "devel/sage/sage/interfaces/gap.py"
Total time for all tests: 29.7 seconds
</pre><p>
That's rather strange, but I guess that's the problem of the GAP spkg.
</p>
<p>
In any case, I don't see a segfault. Not on my laptop, not on the machine in my office, not on bsd.math.
</p>
TicketSimonKingTue, 18 Sep 2012 09:56:51 GMT
https://trac.sagemath.org/ticket/12876#comment:97
https://trac.sagemath.org/ticket/12876#comment:97
<p>
Meanwhile <a class="closed ticket" href="https://trac.sagemath.org/ticket/11521" title="defect: Use weak references to cache homsets (closed: fixed)">#11521</a> has a new dependency, namely <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a>, that is supposed to fix the segfaults (which apparently came from a wrong refcount in libsingular).
</p>
<p>
I am now running <code>make ptest</code> on bsd.math.washington.edu, with
</p>
<pre class="wiki">$ hg qa
trac_715_combined.patch
trac_715_local_refcache.patch
trac_715_safer.patch
trac_715_specification.patch
trac_11521_homset_weakcache_combined.patch
trac_11521_callback.patch
13145.patch
trac_13447sanitise_ring_refcount.patch
trac12215_weak_cached_functionsk.patch
trac12215_segfault_fixes.patch
trac_12313mono_dictcombinedrandomsk.patch
trac_12313_quit_sage.patch
trac13370_deprecate_is_field.patch
trac_13378convert_map_shortcut.patch
trac_13412_category_for_power_series_rings.patch
trac_12876_category_abstract_classes_for_hom.patch
trac_12876_r_test.patch
</pre><p>
applied on top of sage5.4.beta0.
</p>
TicketSimonKingTue, 18 Sep 2012 10:14:07 GMT
https://trac.sagemath.org/ticket/12876#comment:98
https://trac.sagemath.org/ticket/12876#comment:98
<p>
Hooray, all tests pass :)
</p>
TicketnthieryTue, 18 Sep 2012 10:21:44 GMT
https://trac.sagemath.org/ticket/12876#comment:99
https://trac.sagemath.org/ticket/12876#comment:99
<p>
Nice!
</p>
<p>
Anything left to be done to complete the review?
</p>
TicketSimonKingTue, 18 Sep 2012 11:46:58 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/12876#comment:100
https://trac.sagemath.org/ticket/12876#comment:100
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>work_issues</strong>
<em>Understand why the crash in infinite_polynomial_ring is not fixed and what happens with r</em> deleted
</li>
</ul>
<p>
The work issue was "understand the crash of r". I think it <em>is</em> now understood, because it could be solved by using <code>os.realpath</code> in some test, where apparently a relative path was stored.
</p>
<p>
All segfaults that I have seen are gone because of <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/13447" title="defect: Make libsingular multivariate polynomial rings collectable (needs_work)">#13447</a>. But it would be nice if Volker's patchbot could try it again, because it seems to have a tendency to break in a different way than my machines.
</p>
<p>
I have read the patch carefully in the past, and all tests pass, with the patches stated in <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:97" title="Comment 97">comment:97</a>.
</p>
<p>
Hence: Positive review, I'd say.
</p>
TicketjdemeyerWed, 19 Sep 2012 06:30:46 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:101
https://trac.sagemath.org/ticket/12876#comment:101
<ul>
<li><strong>dependencies</strong>
changed from <em>#11521, #12215, #12313, #13412. #13145</em> to <em>#715, #11521, #12215, #12313, #13412. #13145</em>
</li>
</ul>
TicketjdemeyerWed, 19 Sep 2012 06:33:02 GMTdependencies, milestone changed
https://trac.sagemath.org/ticket/12876#comment:102
https://trac.sagemath.org/ticket/12876#comment:102
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215, #12313, #13412. #13145</em> to <em>#13447, #715, #11521, #12215, #12313, #13412. #13145</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage5.4</em> to <em>sagepending</em>
</li>
</ul>
TicketnthierySat, 11 May 2013 03:30:33 GMTstatus, dependencies changed; work_issues set
https://trac.sagemath.org/ticket/12876#comment:103
https://trac.sagemath.org/ticket/12876#comment:103
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>dependencies</strong>
changed from <em>#13447, #715, #11521, #12215, #12313, #13412. #13145</em> to <em>#715, #11521, #12215, #12313, #13412. #13145</em>
</li>
<li><strong>work_issues</strong>
set to <em>needs rebase</em>
</li>
</ul>
TicketnthierySat, 11 May 2013 03:32:51 GMT
https://trac.sagemath.org/ticket/12876#comment:104
https://trac.sagemath.org/ticket/12876#comment:104
<p>
As per the discussion on <a class="new ticket" href="https://trac.sagemath.org/ticket/13347" title="task: Check doctest examples using QuotientRings, which do not fulfill the ... (new)">#13347</a>, I have removed this dependency.
</p>
<p>
The patch needs to be rebased upon <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> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/13184" title="defect: Some Homset are not unique parents (closed: fixed)">#13184</a> that have been merged in 5.10beta0. I am working on it.
</p>
TicketnthierySat, 11 May 2013 04:20:43 GMT
https://trac.sagemath.org/ticket/12876#comment:105
https://trac.sagemath.org/ticket/12876#comment:105
<p>
I just uploaded a rebased patch. This patch sounds almost good to go.
</p>
<p>
Still, there are a couple failing tests which all boil to the following little design issue:
</p>
<p>
Take as typical situation A to be Rings().parent_class, and B to be some <a class="missing wiki">NumberField?</a> parent class. Assume that P is an instance of A resp. B and that:
</p>
<ul><li>A implements _Hom_ so that P._Hom_(codomain) is a <a class="missing wiki">RingHomset?</a> if codomain is a ring
</li><li>B implements _Hom_ so that P._Hom_(codomain) is a <a class="missing wiki">NumberFieldHomset?</a> if codomain is a <a class="missing wiki">NumberField?</a>
</li></ul><p>
Let P be an instance of B and codomain a ring. With the current logic Hom(P, codomain) will
try the later, which will fail, but won't proceed to try the former. So the result will be a plain Homset, and not a <a class="missing wiki">RingHomset?</a> as desired.
</p>
<p>
For some reasons this situation did not appear a year ago. Presumably some tests were added in the mean time. Or we did not run the tests with long ...
</p>
<p>
What's the best way out?
</p>
<ul><li>Option 1: Walk the mro of the class of P, and call cls._Hom_(P, codomain) in turn until something does not fail
</li></ul><ul><li>Option 2: change the protocol so that the _Hom_ method should call super instead of failing if they cannot handle the situation
</li></ul><ul><li>Option 3: ???
</li></ul><p>
Cheers,
</p>
<blockquote>
<p>
Nicolas
</p>
</blockquote>
TicketnthierySat, 11 May 2013 04:21:19 GMTwork_issues changed
https://trac.sagemath.org/ticket/12876#comment:106
https://trac.sagemath.org/ticket/12876#comment:106
<ul>
<li><strong>work_issues</strong>
changed from <em>needs rebase</em> to <em>failing tests; design issue</em>
</li>
</ul>
TicketnthierySat, 11 May 2013 14:17:57 GMTstatus changed; work_issues deleted
https://trac.sagemath.org/ticket/12876#comment:107
https://trac.sagemath.org/ticket/12876#comment:107
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>work_issues</strong>
<em>failing tests; design issue</em> deleted
</li>
</ul>
<p>
Two failures were actually trivial:
</p>
<ul><li>A trivial doctest in
devel/sage/doc/en/thematic_tutorials/coercion_and_categories.rst
</li><li>More skips were needed in the new <a class="missing wiki">TestSuite?</a>(H) in line 114 of sage.categories.hecke_modules
(some new test methods that require the currently missing an_element were added since last year)
</li></ul><p>
I implemented a simplistic protocol for the above issue. If P._Hom_ fails, then try category.parent_class._Hom_. This is sufficient for the other failing test in Parent.
</p>
<p>
Other than this, the attached patch is a straightforward rebase of the previous one. So to finalize the review it would suffice to:
</p>
<ul><li>glance at the above change
</li><li>recheck sage.categories.homset.Hom
</li></ul><p>
I am currently running the long tests, but I expect everything to pass.
</p>
<p>
Almost done!
</p>
TicketnthierySat, 11 May 2013 14:38:11 GMT
https://trac.sagemath.org/ticket/12876#comment:108
https://trac.sagemath.org/ticket/12876#comment:108
<p>
make ptestlong passed smoothly on my machine!
</p>
TicketSimonKingThu, 23 May 2013 17:29:24 GMT
https://trac.sagemath.org/ticket/12876#comment:109
https://trac.sagemath.org/ticket/12876#comment:109
<p>
Are there dependencies missing? With sage5.9.rc0, which should contain all the dependencies listed, I get
</p>
<pre class="wiki">> hg qimport P http://trac.sagemath.org/sage_trac/rawattachment/ticket/12876/trac_12876_category_abstract_classes_for_hom.patch
Füge trac_12876_category_abstract_classes_for_hom.patch zur Seriendatei hinzu
Wende trac_12876_category_abstract_classes_for_hom.patch an
Wende Patch auf Datei doc/en/thematic_tutorials/coercion_and_categories.rst an
FEHLSCHLAG von Teilstück #1 in Zeile 407
1 von 1 Teilstücken sind FEHLGESCHLAGEN  speichere Ausschuss in Datei doc/en/thematic_tutorials/coercion_and_categories.rst.rej
Wende Patch auf Datei sage/categories/homset.py an
Teilstück #1 wurde erfolgreich in Zeile 69 mit Unschärfe 2 angewandt (0 Zeilen verschoben).
FEHLSCHLAG von Teilstück #4 in Zeile 181
FEHLSCHLAG von Teilstück #7 in Zeile 273
2 von 12 Teilstücken sind FEHLGESCHLAGEN  speichere Ausschuss in Datei sage/categories/homset.py.rej
Wende Patch auf Datei sage/modules/vector_space_homspace.py an
FEHLSCHLAG von Teilstück #1 in Zeile 327
1 von 1 Teilstücken sind FEHLGESCHLAGEN  speichere Ausschuss in Datei sage/modules/vector_space_homspace.py.rej
Patch schlug fehl und Fortsetzung unmöglich (versuche v)
Patch schlug fehl, Fehlerabschnitte noch im Arbeitsverzeichnis
Fehler beim Anwenden. Bitte beheben und trac_12876_category_abstract_classes_for_hom.patch aktualisieren
</pre><p>
Hence, several failures in applying the first patch.
</p>
<p>
It seems that the patchbot applies the patches in the wrong order anyway (and also gets a failure).
</p>
<p>
To let the patchbot get it right:
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch trac_12876_r_test.patch
</p>
TicketnthieryThu, 23 May 2013 18:28:37 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:110
https://trac.sagemath.org/ticket/12876#comment:110
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215, #12313, #13412. #13145</em> to <em>#715, #11521, #12215, #12313, #13412, #13145, #14159, #13184</em>
</li>
</ul>
<p>
Yes; I rebased the patch upon <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> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/13184" title="defect: Some Homset are not unique parents (closed: fixed)">#13184</a> which have been merged in 5.10 beta0. I just added them to the dependency list for the record.
</p>
TicketSimonKingThu, 23 May 2013 19:57:58 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:111
https://trac.sagemath.org/ticket/12876#comment:111
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215, #12313, #13412, #13145, #14159, #13184</em> to <em>#715, #11521, #12215, #12313, #13412, #13145, #14159, #13184, #14287</em>
</li>
</ul>
<p>
<a class="closed ticket" href="https://trac.sagemath.org/ticket/14287" title="enhancement: Split _test_elements_eq (closed: fixed)">#14287</a> is also needed. With this, the patch applies.
</p>
TicketSimonKingThu, 23 May 2013 19:58:57 GMT
https://trac.sagemath.org/ticket/12876#comment:112
https://trac.sagemath.org/ticket/12876#comment:112
<p>
However, there seems to be yet another dependency for the second patch.
</p>
TicketSimonKingThu, 23 May 2013 20:02:19 GMT
https://trac.sagemath.org/ticket/12876#comment:113
https://trac.sagemath.org/ticket/12876#comment:113
<p>
It fails to apply, because in r.py it reads
</p>
<pre class="wiki"> sage: os.path.realpath(tmpdir) == sageobj(r.getwd()) # known bug (:trac:`9970`)
</pre><p>
but the second patch from here assumes that there is no pointer to <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a>.
</p>
TicketSimonKingThu, 23 May 2013 20:03:25 GMT
https://trac.sagemath.org/ticket/12876#comment:114
https://trac.sagemath.org/ticket/12876#comment:114
<p>
The pointer got introduced in <a class="closed ticket" href="https://trac.sagemath.org/ticket/12415" title="enhancement: Update doctesting framework (closed: fixed)">#12415</a>, as it seems.
</p>
TicketSimonKingThu, 23 May 2013 20:13:28 GMT
https://trac.sagemath.org/ticket/12876#comment:115
https://trac.sagemath.org/ticket/12876#comment:115
<p>
After applying the failing hunk manually, I am now running make ptestlong.
</p>
TicketkcrismanThu, 23 May 2013 20:26:56 GMT
https://trac.sagemath.org/ticket/12876#comment:116
https://trac.sagemath.org/ticket/12876#comment:116
<p>
I don't think that the reference to <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a> should be removed. It's okay to put in a different test for now if that helps things, but it doesn't remove the underlying problem in our parsing that wjp has a tentative patch for there, so we shouldn't just completely not mention it. See <a class="extlink" href="http://trac.sagemath.org/sage_trac/ticket/9970#comment:20"><span class="icon"></span>this comment</a> by Simon for background.
</p>
TicketSimonKingThu, 23 May 2013 20:45:38 GMT
https://trac.sagemath.org/ticket/12876#comment:117
https://trac.sagemath.org/ticket/12876#comment:117
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:116" title="Comment 116">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
I don't think that the reference to <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a> should be removed. It's okay to put in a different test for now if that helps things, but it doesn't remove the underlying problem in our parsing that wjp has a tentative patch for there, so we shouldn't just completely not mention it. See <a class="extlink" href="http://trac.sagemath.org/sage_trac/ticket/9970#comment:20"><span class="icon"></span>this comment</a> by Simon for background.
</p>
</blockquote>
<p>
In any case, the second patch needs to be rebased against <a class="closed ticket" href="https://trac.sagemath.org/ticket/12415" title="enhancement: Update doctesting framework (closed: fixed)">#12415</a>, right?
</p>
TicketnthieryThu, 23 May 2013 21:05:57 GMT
https://trac.sagemath.org/ticket/12876#comment:118
https://trac.sagemath.org/ticket/12876#comment:118
<p>
Simon: are you sure we need trac_12876_r_test.patch? It's a patch that you had added and actually I forgot to insert it in the sagecombinat queue; and all tests are passing smoothly for me without it!
</p>
TicketSimonKingThu, 23 May 2013 21:14:00 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:119
https://trac.sagemath.org/ticket/12876#comment:119
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=119">diff</a>)
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:118" title="Comment 118">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Simon: are you sure we need trac_12876_r_test.patch? It's a patch that you had added and actually I forgot to insert it in the sagecombinat queue; and all tests are passing smoothly for me without it!
</p>
</blockquote>
<p>
Really?
</p>
<p>
OK, then let's try:
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
TicketkcrismanFri, 24 May 2013 01:33:18 GMT
https://trac.sagemath.org/ticket/12876#comment:120
https://trac.sagemath.org/ticket/12876#comment:120
<blockquote class="citation">
<p>
It's a patch that you had added and actually I forgot to insert it in the sagecombinat queue; and all tests are passing smoothly for me without it!
</p>
</blockquote>
<p>
This is a pretty unstable situation  it doesn't appear all the time and not on all platforms. I think that the idea of replacing this with a test that works for now is fine  see Simon's suggestion at <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a>.
</p>
TicketnthieryFri, 24 May 2013 02:06:56 GMT
https://trac.sagemath.org/ticket/12876#comment:121
https://trac.sagemath.org/ticket/12876#comment:121
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:120" title="Comment 120">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
This is a pretty unstable situation  it doesn't appear all the time and not on all platforms. I think that the idea of replacing this with a test that works for now is fine  see Simon's suggestion at <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a>.
</p>
</blockquote>
<p>
I am fine with that. I just still need to understand at some point how this is related to this ticket :)
</p>
TicketnthieryFri, 24 May 2013 02:25:09 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876schemes_homcategory.patch</em>
</li>
</ul>
TicketnthieryFri, 24 May 2013 02:26:51 GMT
https://trac.sagemath.org/ticket/12876#comment:122
https://trac.sagemath.org/ticket/12876#comment:122
<p>
With the attached extra patch, all test still pass and there is no more crappy <span class="underline">new</span> in Schemes.<a class="missing wiki">HomCategory?</a>.<a class="missing wiki">ParentMethods?</a>.
</p>
TicketnthieryFri, 24 May 2013 02:27:40 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:123
https://trac.sagemath.org/ticket/12876#comment:123
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=123">diff</a>)
</li>
</ul>
TicketSimonKingFri, 24 May 2013 04:51:29 GMT
https://trac.sagemath.org/ticket/12876#comment:124
https://trac.sagemath.org/ticket/12876#comment:124
<p>
Patchbot times out at sage/interfaces/ecm.py. However, with the first two patches, make ptestlong passes.
</p>
<p>
My plan is to modify the second patch, so that reference to <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/9970" title="defect: Flaky doctest in sage/interfaces/r.py (needs_work)">#9970</a> is preserved, and to repeat make ptestlong with the third patch.
</p>
TicketSimonKingFri, 24 May 2013 05:17:17 GMT
https://trac.sagemath.org/ticket/12876#comment:125
https://trac.sagemath.org/ticket/12876#comment:125
<p>
What other dependency is there for the third patch?
</p>
TicketSimonKingFri, 24 May 2013 05:22:27 GMT
https://trac.sagemath.org/ticket/12876#comment:126
https://trac.sagemath.org/ticket/12876#comment:126
<p>
I have
</p>
<pre class="wiki">> hg qa
trac_14159_weak_value_triple_dict.patch
trac_14159_use_cdef_get.patch
trac_13184_sage_5.9.beta.patch
trac_14287rebased.patch
trac_12876_category_abstract_classes_for_hom.patch
trac_12876_r_test.patch
</pre><p>
on top of sage5.9.rc0, and the relevant lines in sage/categories/schemes.py are
</p>
<pre class="wiki"> """
return []
class ParentMethods:
def __new__(cls, R, S, category):
"""
TESTS::
sage: E = EllipticCurve('37a1')
sage: Hom(E, E).__class__
<class 'sage.schemes.generic.homset.SchemeHomset_generic_with_category'>
If both schemes R and S are actually specs, we want
the parent for Hom(R, S) to be in a different class::
sage: Hom(Spec(ZZ), Spec(ZZ)).__class__
<class 'sage.schemes.generic.homset.SchemeHomset_points_spec_with_category'>
Currently, and to minimize the changes, this is done
by delegating the job to SchemeHomset. This is not
very robust: for example, only one category can do
this hack.
FIXME: this might be better handled by an extra Spec category
"""
from sage.schemes.generic.homset import SchemeHomset
return SchemeHomset(R, S, category=category)
#############################################################
</pre><p>
However, the patch expects
</p>
<pre class="wiki"> """
return []
 class ParentMethods:

 def __new__(cls, R, S, category):
 """
 TESTS::

 sage: E = EllipticCurve('37a1')
 sage: Hom(E, E).__class__
 <class 'sage.schemes.generic.homset.SchemeHomset_generic_with_category'>

 If both schemes R and S are actually specs, we want
 the parent for Hom(R, S) to be in a different class::

 sage: Hom(Spec(ZZ), Spec(ZZ)).__class__
 <class 'sage.schemes.affine.affine_homset.SchemeHomset_points_spec_with_category'>

 Currently, and to minimize the changes, this is done
 by delegating the job to SchemeHomset. This is not
 very robust: for example, only one category can do
 this hack.

 FIXME: this might be better handled by an extra Spec category
 """
 from sage.schemes.generic.homset import SchemeHomset
 return SchemeHomset(R, S, category=category)
#############################################################
</pre><p>
So, what tickect changed <code>sage.schemes.generic.homset.SchemeHomset_points_spec</code> into <code>sage.schemes.affine.affine_homset.SchemeHomset_points_spec</code>?
</p>
TicketSimonKingFri, 24 May 2013 05:26:39 GMTdependencies changed
https://trac.sagemath.org/ticket/12876#comment:127
https://trac.sagemath.org/ticket/12876#comment:127
<ul>
<li><strong>dependencies</strong>
changed from <em>#715, #11521, #12215, #12313, #13412, #13145, #14159, #13184, #14287</em> to <em>#715, #11521, #12215, #12313, #13412, #13145, #14159, #13184, #14287, #14217</em>
</li>
</ul>
<p>
It is <a class="closed ticket" href="https://trac.sagemath.org/ticket/14217" title="enhancement: basic iteration functionality for affine and projective morphisms (closed: fixed)">#14217</a>.
</p>
TicketSimonKingFri, 24 May 2013 05:32:09 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_r_test.patch</em>
</li>
</ul>
<p>
Try to make one test in interfaces/r.py more stable wrt symbolic links
</p>
TicketSimonKingFri, 24 May 2013 05:34:32 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:128
https://trac.sagemath.org/ticket/12876#comment:128
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=128">diff</a>)
</li>
</ul>
<p>
Patchbot:
Apply trac_12876_category_abstract_classes_for_hom.patch trac_12876_r_test.py trac_12876schemes_homcategory.patch
</p>
TicketjdemeyerFri, 24 May 2013 06:34:08 GMT
https://trac.sagemath.org/ticket/12876#comment:129
https://trac.sagemath.org/ticket/12876#comment:129
<p>
I really don't like <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_r_test.patch" title="Attachment 'trac_12876_r_test.patch' in Ticket #12876">trac_12876_r_test.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_r_test.patch" title="Download"></a>. Are we sure the test actually works now (you removed <code># known bug</code>)?
</p>
TicketjdemeyerFri, 24 May 2013 06:39:27 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:130
https://trac.sagemath.org/ticket/12876#comment:130
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=130">diff</a>)
</li>
</ul>
TicketjdemeyerFri, 24 May 2013 06:42:14 GMT
https://trac.sagemath.org/ticket/12876#comment:131
https://trac.sagemath.org/ticket/12876#comment:131
<p>
Given that the other patches don't change <strong>anything</strong> in the R interface, it is almost impossible that this ticket could have any influence on the R test. Or at least, it requires a good explanation if something did change.
</p>
<p>
Proposal: do not apply <a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_r_test.patch" title="Attachment 'trac_12876_r_test.patch' in Ticket #12876">trac_12876_r_test.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_r_test.patch" title="Download"></a>
</p>
TicketjdemeyerFri, 24 May 2013 06:42:26 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:132
https://trac.sagemath.org/ticket/12876#comment:132
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
TicketSimonKingFri, 24 May 2013 06:54:46 GMT
https://trac.sagemath.org/ticket/12876#comment:133
https://trac.sagemath.org/ticket/12876#comment:133
<p>
I'm fine with not applying the rtest patch. An info that I need: Does that tag <code># known bug</code> mean that the patchbot will not complain about it?
</p>
TicketSimonKingFri, 24 May 2013 06:56:56 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:134
https://trac.sagemath.org/ticket/12876#comment:134
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=134">diff</a>)
</li>
</ul>
<p>
Patchbot:
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch trac_12876schemes_homcategory.patch
</p>
TicketjdemeyerFri, 24 May 2013 06:59:28 GMT
https://trac.sagemath.org/ticket/12876#comment:135
https://trac.sagemath.org/ticket/12876#comment:135
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:133" title="Comment 133">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Does that tag <code># known bug</code> mean that the patchbot will not complain about it?
</p>
</blockquote>
<p>
Yes, see <a class="extlink" href="http://sagemath.org/doc/developer/conventions.html#furtherconventionsforautomatedtestingofexamples"><span class="icon"></span>http://sagemath.org/doc/developer/conventions.html#furtherconventionsforautomatedtestingofexamples</a>
</p>
TicketjdemeyerFri, 24 May 2013 06:59:44 GMTmilestone changed
https://trac.sagemath.org/ticket/12876#comment:136
https://trac.sagemath.org/ticket/12876#comment:136
<ul>
<li><strong>milestone</strong>
changed from <em>sagepending</em> to <em>sage5.10</em>
</li>
</ul>
TicketnthieryFri, 24 May 2013 10:43:55 GMT
https://trac.sagemath.org/ticket/12876#comment:137
https://trac.sagemath.org/ticket/12876#comment:137
<p>
Hi Simon!
</p>
<blockquote class="citation">
<p>
on top of sage5.9.rc0, and the relevant lines in sage/categories/schemes.py are
</p>
</blockquote>
<p>
Time for an upgrade maybe? :)
</p>
TicketnthieryFri, 24 May 2013 10:46:02 GMT
https://trac.sagemath.org/ticket/12876#comment:138
https://trac.sagemath.org/ticket/12876#comment:138
<p>
Patchbot is green :)
</p>
TicketSimonKingFri, 24 May 2013 10:57:07 GMT
https://trac.sagemath.org/ticket/12876#comment:139
https://trac.sagemath.org/ticket/12876#comment:139
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:138" title="Comment 138">nthiery</a>:
</p>
<blockquote class="citation">
<p>
Patchbot is green :)
</p>
</blockquote>
<p>
Yes, I know. I need some more time to finish yet another job application.
</p>
TicketnthieryFri, 24 May 2013 11:05:09 GMT
https://trac.sagemath.org/ticket/12876#comment:140
https://trac.sagemath.org/ticket/12876#comment:140
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:139" title="Comment 139">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Yes, I know. I need some more time to finish yet another job application.
</p>
</blockquote>
<p>
I did not mean to pressure; I was just happy :)
</p>
<p>
Of course job applications have priority! Good luck!
</p>
TicketSimonKingFri, 24 May 2013 11:59:05 GMT
https://trac.sagemath.org/ticket/12876#comment:141
https://trac.sagemath.org/ticket/12876#comment:141
<p>
From the commit message:
</p>
<ul><li>If category is None, Hom simply calls itself with the meet of the
categories of the parent, which removes a cache handling duplication.
</li></ul><p>
I don't know if that's a good idea. Duplicating the cache costs virtually nothing (because all objects, categories and homsets involved are there anyway), but it speeds up the most commonly used case, namely category=None. Computing the meet of the underlying categories is expensive, even though it is cached.
</p>
TicketSimonKingFri, 24 May 2013 12:15:51 GMT
https://trac.sagemath.org/ticket/12876#comment:142
https://trac.sagemath.org/ticket/12876#comment:142
<p>
Question: Is it really needed that each category defines its own <code>HomCategory</code>, which in most cases is just
</p>
<pre class="wiki"> class HomCategory(HomCategory):
pass
</pre><p>
?
</p>
<p>
Since all classes for categories inherit from <code>Category</code>, couldn't one just define the <code>HomCategory</code> there, and only replace it when really needed?
</p>
<p>
This could be for a different ticket, though. I just wonder.
</p>
TicketSimonKingFri, 24 May 2013 12:17:31 GMT
https://trac.sagemath.org/ticket/12876#comment:143
https://trac.sagemath.org/ticket/12876#comment:143
<p>
BTW, thank you for fixing the coercion cache of <code>InfinitePolynomialRing</code>. Now, as the default cache is weak anyway, the custom cache can certainly be removed now.
</p>
TicketSimonKingFri, 24 May 2013 12:22:25 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:144
https://trac.sagemath.org/ticket/12876#comment:144
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
Tests passed. I went through both patches, and they look mostly good, except for the minor criticism in <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:141" title="Comment 141">comment:141</a> and <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:142" title="Comment 142">comment:142</a>. It is almost a positive review, but perhaps you have a comment on these two points.
</p>
TicketnthieryFri, 24 May 2013 13:32:00 GMT
https://trac.sagemath.org/ticket/12876#comment:145
https://trac.sagemath.org/ticket/12876#comment:145
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:141" title="Comment 141">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
From the commit message:
</p>
<ul><li>If category is None, Hom simply calls itself with the meet of the
categories of the parent, which removes a cache handling duplication.
</li></ul><p>
I don't know if that's a good idea. Duplicating the cache costs
virtually nothing (because all objects, categories and homsets
involved are there anyway), but it speeds up the most commonly used
case, namely category=None. Computing the meet of the underlying
categories is expensive, even though it is cached.
</p>
</blockquote>
<p>
Oh you are right. I meant to avoid a duplication in the source code, not in the caching itself. I agree with you, the current code is wrong. I am going to fix this right now.
</p>
<p>
Thanks for catching this!
</p>
TicketnthieryFri, 24 May 2013 13:53:19 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876fixhomsetcachent.patch</em>
</li>
</ul>
TicketnthieryFri, 24 May 2013 13:56:33 GMTstatus, description changed
https://trac.sagemath.org/ticket/12876#comment:146
https://trac.sagemath.org/ticket/12876#comment:146
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=146">diff</a>)
</li>
</ul>
<p>
The new uploaded patch is the previous one with
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876schemes_homcategory.patch" title="Attachment 'trac_12876schemes_homcategory.patch' in Ticket #12876">trac_12876schemes_homcategory.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876schemes_homcategory.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876fixhomsetcachent.patch" title="Attachment 'trac_12876fixhomsetcachent.patch' in Ticket #12876">trac_12876fixhomsetcachent.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876fixhomsetcachent.patch" title="Download"></a>
</li></ul><p>
folded in. The latter should fix the caching of Hom(X,Y).
</p>
<p>
I am currently running all long tests!
</p>
TicketnthieryFri, 24 May 2013 14:05:05 GMT
https://trac.sagemath.org/ticket/12876#comment:147
https://trac.sagemath.org/ticket/12876#comment:147
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:142" title="Comment 142">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Question: Is it really needed that each category defines its own <code>HomCategory</code>, which in most cases is just
</p>
<pre class="wiki"> class HomCategory(HomCategory):
pass
</pre><p>
?
</p>
<p>
Since all classes for categories inherit from <code>Category</code>, couldn't one just define the <code>HomCategory</code> there, and only replace it when really needed?
</p>
</blockquote>
<p>
Well, most categories actually don't define a <a class="missing wiki">HomCategory?</a>. I did not
strip the now trivial <a class="missing wiki">HomCategory?</a> in Schemes just as to avoid having
to fix some trivial doctest failures elsewhere (without it, a
Hom(scheme,scheme) was downgraded to the category of homsets of sets).
</p>
<blockquote class="citation">
<p>
This could be for a different ticket, though. I just wonder.
</p>
</blockquote>
<p>
Yup. At this point we have very little use cases of <a class="missing wiki">HomCategories?</a>, and
I don't have a good view on how often this gadget will be used, and
thus what the sane default should be. So I would indeed leave things
untouched for now until we have more experience.
</p>
TicketSimonKingFri, 24 May 2013 14:07:29 GMT
https://trac.sagemath.org/ticket/12876#comment:148
https://trac.sagemath.org/ticket/12876#comment:148
<p>
You should also tell the patchbot what to do.
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
TicketnthieryFri, 24 May 2013 14:12:00 GMT
https://trac.sagemath.org/ticket/12876#comment:149
https://trac.sagemath.org/ticket/12876#comment:149
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:143" title="Comment 143">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
BTW, thank you for fixing the coercion cache of <code>InfinitePolynomialRing</code>. Now, as the default cache is weak anyway, the custom cache can certainly be removed now.
</p>
</blockquote>
<p>
Well, actually you wrote this piece; see <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:59" title="Comment 59">59</a>!
</p>
<p>
I am fine with this change. And since you had forgotten that you had
written it in the first place, I consider your selfreview of this
piece as sufficient :)
</p>
TicketnthieryFri, 24 May 2013 14:13:13 GMT
https://trac.sagemath.org/ticket/12876#comment:150
https://trac.sagemath.org/ticket/12876#comment:150
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:148" title="Comment 148">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
You should also tell the patchbot what to do.
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
</blockquote>
<p>
Ah shoot; I had done this in the description but not here. Thanks. I really don't like that the patch bot is extracting this info from the comments rather than the description. Oh well.
</p>
TicketnthieryFri, 24 May 2013 14:34:27 GMT
https://trac.sagemath.org/ticket/12876#comment:151
https://trac.sagemath.org/ticket/12876#comment:151
<p>
All test passed:
</p>
<p>
<a class="extlink" href="http://sage.math.washington.edu/home/nthiery/trac_12876_category_abstract_classes_for_hom.patchtestlog"><span class="icon"></span>http://sage.math.washington.edu/home/nthiery/trac_12876_category_abstract_classes_for_hom.patchtestlog</a>
</p>
<p>
Well, almost. I got the following error:
</p>
<pre class="wiki">sage t long devel/sage/sage/graphs/genus.pyx
**********************************************************************
File "devel/sage/sage/graphs/genus.pyx", line 136, in sage.graphs.genus.simple_connected_genus_backtracker.__dealloc__
Failed example:
get_memory_usage(t) <= 0.0
Expected:
True
Got:
False
</pre><p>
but can't reproduce it; so this sounds more like an unrelated random sporadic unfluke.
</p>
<p>
Let's see what the patchbot says!
</p>
TicketnthieryFri, 24 May 2013 14:35:46 GMT
https://trac.sagemath.org/ticket/12876#comment:152
https://trac.sagemath.org/ticket/12876#comment:152
<p>
It seems to be happy :)
</p>
TicketSimonKingFri, 24 May 2013 14:44:20 GMT
https://trac.sagemath.org/ticket/12876#comment:153
https://trac.sagemath.org/ticket/12876#comment:153
<p>
OK, I am just waiting for my own tests to finish, and will then provide a reviewer patch, adding a test that shows that providing <code>Category.meet([X.category(),Y.category()])</code> is equivalent to providing <code>None</code>.
</p>
TicketSimonKingFri, 24 May 2013 14:45:12 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876review.patch</em>
</li>
</ul>
TicketSimonKingFri, 24 May 2013 14:46:17 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:154
https://trac.sagemath.org/ticket/12876#comment:154
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=154">diff</a>)
</li>
</ul>
<p>
Or better: Add the patch now and let the bot start over.
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch trac_12876review.patch
</p>
TicketnbruinFri, 24 May 2013 15:00:12 GMT
https://trac.sagemath.org/ticket/12876#comment:155
https://trac.sagemath.org/ticket/12876#comment:155
<p>
for the review patch: "identic" > "identical"
</p>
TicketnthieryFri, 24 May 2013 15:06:52 GMT
https://trac.sagemath.org/ticket/12876#comment:156
https://trac.sagemath.org/ticket/12876#comment:156
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12876#comment:155" title="Comment 155">nbruin</a>:
</p>
<blockquote class="citation">
<p>
for the review patch: "identic" > "identical"
</p>
</blockquote>
<p>
Yup. I am on it.
</p>
TicketnthieryFri, 24 May 2013 15:09:34 GMTattachment set
https://trac.sagemath.org/ticket/12876
https://trac.sagemath.org/ticket/12876
<ul>
<li><strong>attachment</strong>
set to <em>trac_12876_category_abstract_classes_for_hom.patch</em>
</li>
</ul>
<p>
Combined patch
</p>
TicketnthieryFri, 24 May 2013 15:10:09 GMTdescription changed
https://trac.sagemath.org/ticket/12876#comment:157
https://trac.sagemath.org/ticket/12876#comment:157
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/12876?action=diff&version=157">diff</a>)
</li>
</ul>
TicketnthieryFri, 24 May 2013 15:10:56 GMT
https://trac.sagemath.org/ticket/12876#comment:158
https://trac.sagemath.org/ticket/12876#comment:158
<p>
I folded in you review patch, and fixed identic to identical (also in a paragraph just after).
</p>
<p>
Apply:
</p>
<ul><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/12876/trac_12876_category_abstract_classes_for_hom.patch" title="Attachment 'trac_12876_category_abstract_classes_for_hom.patch' in Ticket #12876">trac_12876_category_abstract_classes_for_hom.patch</a><a class="tracrawlink" href="https://trac.sagemath.org/rawattachment/ticket/12876/trac_12876_category_abstract_classes_for_hom.patch" title="Download"></a>
</li></ul>
TicketSimonKingFri, 24 May 2013 15:12:51 GMTstatus changed
https://trac.sagemath.org/ticket/12876#comment:159
https://trac.sagemath.org/ticket/12876#comment:159
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
OK, for me, the tests pass. So, I make it a positive review.
</p>
TicketSimonKingFri, 24 May 2013 15:14:12 GMT
https://trac.sagemath.org/ticket/12876#comment:160
https://trac.sagemath.org/ticket/12876#comment:160
<p>
PS, I did briefly check the combined patch...
</p>
<p>
Apply trac_12876_category_abstract_classes_for_hom.patch
</p>
TicketnthieryFri, 24 May 2013 15:18:19 GMT
https://trac.sagemath.org/ticket/12876#comment:161
https://trac.sagemath.org/ticket/12876#comment:161
<p>
Youpi!
</p>
<p>
Thanks Simon!
</p>
TicketjdemeyerMon, 27 May 2013 06:24:45 GMTmilestone changed
https://trac.sagemath.org/ticket/12876#comment:162
https://trac.sagemath.org/ticket/12876#comment:162
<ul>
<li><strong>milestone</strong>
changed from <em>sage5.10</em> to <em>sage5.11</em>
</li>
</ul>
TicketjdemeyerThu, 06 Jun 2013 12:31:56 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/12876#comment:163
https://trac.sagemath.org/ticket/12876#comment:163
<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>merged</strong>
set to <em>sage5.11.beta0</em>
</li>
</ul>
Ticket