#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.4
Component: categories | Resolution:
Keywords: categories, Hom | Work issues: add commit message
to one patch
Report Upstream: N/A | Reviewers: Simon King
Authors: Nicolas M. Thiéry | Merged in:
Dependencies: #11521, #12215, #12313 | Stopgaps:
------------------------------------------+---------------------------------
Changes (by SimonKing):
* work_issues: fix doctests and segfaults => add commit message to one
patch
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-sk-rel11521.patch]
> * [attachment:trac_12876-reviewer.patch]
> * [attachment:trac_12876_category-fix_abstract_class-nt-rel11521-review-
> nt.patch]
> * [attachment:trac_12876_fix_infinite_polynomial_ring.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-sk-rel11521.patch]
* [attachment:trac_12876-reviewer.patch]
* [attachment:trac_12876_category-fix_abstract_class-nt-rel11521-review-
nt.patch]
* [attachment:trac_12876_fix_infinite_polynomial_ring.patch]
* [attachment:trac_12876_another_reviewer.patch]
--
Comment:
I give it a positive review, modulo commit message of [attachment
:trac_12876_category-fix_abstract_class-nt-rel11521-review-nt.patch] and
modulo the small reviewer patch that I have attached. I think it should be
`.. todo::` and not `..TODO::` (note the blank space).
So, when you added a commit message to your patch, then please change it
into a positive review!
Apply trac_12876_category-fix_abstract_class-sk-rel11521.patch
trac_12876-reviewer.patch trac_12876_category-fix_abstract_class-nt-
rel11521-review-nt.patch trac_12876_fix_infinite_polynomial_ring.patch
trac_12876_another_reviewer.patch
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12876#comment:71>
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.
