#12876: Fix element and parent classes of Hom categories to be abstract, and
simplify the Hom logic.
--------------------------------------------------------+-------------------
Reporter: nthiery | Owner:
nthiery
Type: enhancement | Status:
needs_review
Priority: major | Milestone:
sage-5.1
Component: categories | Resolution:
Keywords: categories, Hom | Work issues: Fix
ring_refcount_dict problem
Report Upstream: N/A | Reviewers: Simon
King
Authors: Nicolas M. Thiéry | Merged in:
Dependencies: #715, #11521, #12875, #12877, #12215 | Stopgaps:
--------------------------------------------------------+-------------------
Changes (by SimonKing):
* dependencies: #12875, #12877 => #715, #11521, #12875, #12877, #12215
Old description:
> This patch fixes the parent and element classes for Hom categories to
> be purely abstract, and simplifies the Hom logic:
>
> - Unified the logic for selecting the class when building a Homset
> (e.g. Homset, RingHomset, HeckeModuleHomspace, ...). This is now
> systematically done through the _Hom_ hook. The logic still has a
> fundamental flaw, but that's for the later #10668.
> - The cache for Hom is handled at a single point in Hom
> In particular, homsets created via the _Hom_ hook are now unique.
> - If category is None, Hom simply calls itself with the meet of the
> categories of the parent, which removes a cache handling duplication.
> - Parent.Hom calls Hom directly (removes duplicate _Hom_ logic).
> - ParentWithBase.Hom was redundant and is gone.
> - 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.
> - Update a doctest in sage.modules.vector_space_homspace to take into
> account that homsets created via _Hom_ are now unique.
> - Scheme is (apparently) an abstract base class; so it should not be
> instantiated. I changed some doctests in
> sage.schemes.generic.SchemeMorphism to use instead the concrete
> Spec(ZZ). Those doctests were breaking because Scheme does not
> implement equality, which is required for Hom caching.
>
> As a byproduct, the HeckeModules category does not import any more
> HeckeModulesHomspace, which was a recurrent source of import loops.
>
> #11935 depends on this ticket
>
> Apply:
>
> [attachment:trac_12876_category-fix_abstract_class-nt-rel11521.patch]
New description:
This patch fixes the parent and element classes for Hom categories to
be purely abstract, and simplifies the Hom logic:
- Unified the logic for selecting the class when building a Homset
(e.g. Homset, RingHomset, HeckeModuleHomspace, ...). This is now
systematically done through the _Hom_ hook. The logic still has a
fundamental flaw, but that's for the later #10668.
- The cache for Hom is handled at a single point in Hom
In particular, homsets created via the _Hom_ hook are now unique.
- If category is None, Hom simply calls itself with the meet of the
categories of the parent, which removes a cache handling duplication.
- Parent.Hom calls Hom directly (removes duplicate _Hom_ logic).
- ParentWithBase.Hom was redundant and is gone.
- 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.
- Update a doctest in sage.modules.vector_space_homspace to take into
account that homsets created via _Hom_ are now unique.
- Scheme is (apparently) an abstract base class; so it should not be
instantiated. I changed some doctests in
sage.schemes.generic.SchemeMorphism to use instead the concrete
Spec(ZZ). Those doctests were breaking because Scheme does not
implement equality, which is required for Hom caching.
As a byproduct, the HeckeModules category does not import any more
HeckeModulesHomspace, which was a recurrent source of import loops.
#11935 depends on this ticket
Apply:
* [attachment:trac_12876_category-fix_abstract_class-nt-rel11521.patch]
* [attachment:trac_12876-reviewer.patch]
--
Comment:
With sage-5.0.beta13 plus #715, #11521, #12875, #12877, #12215 and
[attachment:trac_12876_category-fix_abstract_class-nt-rel11521.patch], all
doc tests pass.
I made some additional cosmetic changes in a reviewer patch.
Further questions:
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.
I see that you rename `Rings.HomCategory.ParentMethods.__new__` into
`__new__bx`. Does that mean the ugly `__new__` method ought to be removed
(finally!)?
But then, shouldn't `__getnewargs__` be removed as well?
Please tell whether you want to address these questions in an additional
patch. And I hope you agree with adding #715, #11521 and #12215 to the
dependencies? Only #12215 needs review (hint...).
If you do agree, then put this to ''positive review'', please.
Apply trac_12876_category-fix_abstract_class-nt-rel11521.patch
trac_12876-reviewer.patch
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12876#comment:22>
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.
