Opened 8 years ago
Last modified 7 years ago
#12876 closed enhancement
Fix element and parent classes of Hom categories to be abstract, and simplify the Hom logic. — at Version 5
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-5.11 |
Component: | categories | Keywords: | categories, Hom |
Cc: | sage-combinat, SimonKing | Merged in: | |
Authors: | Nicolas M. Thiéry | Reviewers: | Simon King |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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
Change History (5)
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
- Description modified (diff)
comment:3 Changed 8 years ago by
- Reviewers set to Simon King
Note: there are two doctests failures that I don't know how to handle, related to the refcounting of Singular rings::
sage -t devel/sage-combinat/sage/rings/polynomial/multi_polynomial_libsingular.pyx ********************************************************************** File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/rings/polynomial/multi_polynomial_libsingular.pyx", line 418: sage: len(ring_refcount_dict) == n Expected: True Got: False sage -t devel/sage-combinat/sage/libs/singular/ring.pyx ********************************************************************** File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/libs/singular/ring.pyx", line 469: sage: ring_ptr in ring_refcount_dict Expected: False Got: True
Help fixing those welcome!
comment:4 Changed 8 years ago by
The updated patch includes two small related improvements I had in later patches. I moved them here to resolve the conflict.
comment:5 Changed 8 years ago by
- Description modified (diff)
Note: See
TracTickets for help on using
tickets.
Note: this might depend on #12875