#11935: Make parent/element classes independent of base rings
-----------------------------------------------------------------+----------
Reporter: SimonKing |
Owner: nthiery
Type: enhancement |
Status: needs_review
Priority: major |
Milestone: sage-5.0
Component: categories |
Resolution:
Keywords: parent class, element class, category | Work
issues:
Report Upstream: N/A |
Reviewers:
Authors: Simon King | Merged
in:
Dependencies: #9138, #11900, #11943, #12875, #12876, #12877 |
Stopgaps:
-----------------------------------------------------------------+----------
Comment (by nthiery):
Hi Simon,
While working on #12895, I got a non trivial time regression, due to the
large number of constructed categories for which creating yet another
*_class was non negligible. Investigating this with runsnake made me turn
back to this ticket: too many categories are created for nothing.
It indeed sounds a bit like a waste, to construct the parent class of
Algebras(GF(5)) to have to reconstruct all the hierarchy of super
categories above Algebras(GF(5)) (e.g. Modules(GF(5)), ...). With the
updated patch, Algebras(K).parent_class directly reuses
Algebras(L).parent_class if it already exists and if K and L have the same
category. The super_categories method of Algebras(K) is not even called.
To achieve this, each subcategory of CategoryWithParameters should provide
a method _make_named_class_key specifying on what the parent_class (and
friends) depend on. For example, Category_over_base specifies that
parent_class depends only on the category of the base.
Then, _make_named_class uses that to do a lookup in a cache.
For our typical benchmark:
{{{
%time L = EllipticCurve('960d1').prove_BSD()
}}}
the time on my machine goes from 4s down to 3.5s. With the subcategory
patch, the times goes down from 7s to 3.75s. This makes the subcategory
patch acceptable.
One fine point is that e.g. Algebras(ZZ) and Algebras(ZZ['x']) don't share
the same parent class anymore, since ZZ and ZZ['x'] don't have the same
category.
What do you think? Could you have a brief look at the experimental
trac11935_share_on_base_category.patch I just attached? If it sounds
reasonable to you, I'll finalize it (doctests, ...), and fold it in my
reviewer's patch.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11935#comment:53>
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.
