#11900: Serious regression caused by #9138
------------------------------+---------------------------------------------
Reporter: SimonKing | Owner: tbd
Type: defect | Status: needs_work
Priority: critical | Milestone: sage-4.8
Component: performance | Keywords: categories regression
Work_issues: | Upstream: N/A
Reviewer: Jeroen Demeyer | Author: Simon King
Merged: | Dependencies: #9138 #11911
------------------------------+---------------------------------------------
Comment(by nthiery):
Replying to [comment:98 SimonKing]:
> Hi Nicolas, it seems that your "singleton" patch does not contain the
pyx file.
Oops! Fixed!
> By the way, since is_subcategory is used by `__contains__`, I would
somehow prefer to not cythonise `__contains__` but cythonise
`is_subcategory` instead.
I see your point.
However, one would need to use the subcategory_hook: in `x in
Rings()`, the is_subcategory that is called is that of the category of
x, not of Rings(). So altogether it might be not as fast as when
cythonizing __contains__. Also, since the optimized code for
__contains__ is small, I can live with the tiny code duplication due
to the "manual inlining of is_subcategory" in __contains__.
> Anyway. Since the change concerns "speedup for category framework", the
new stuff shall remain on ''this'' ticket. Do we agree on that?
Definitely.
> Of course, after cythonising is_subcategory, I need to change my patches
from #11943 and #11935 as well. In particular #11935 will be tricky,
because the idea of "testing subcategories via subclasses of the parent
class" will not work if parent classes are shared. But that problem shall
be dealt with there and not here.
Ah, I see, you are thinking of cythonizing is_subcategories via
parent_class for *all* categories. Hmm, I haven't yet made up my mind
about this. Unless it brings a clear general improvement (and not only
for the specific regression this patch is about), shalle we not postpone
that to later, and stick to a more local change (only about Rings and
friends)?
Cheers,
Nicolas
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11900#comment:99>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
