Opened 10 years ago

Last modified 9 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:

Status badges

Description (last modified by nthiery)

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 10 years ago by nthiery

  • Status changed from new to needs_review

Note: this might depend on #12875

comment:2 Changed 10 years ago by nthiery

  • Description modified (diff)

comment:3 Changed 10 years ago by nthiery

  • 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
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

Help fixing those welcome!

comment:4 Changed 10 years ago by nthiery

The updated patch includes two small related improvements I had in later patches. I moved them here to resolve the conflict.

comment:5 Changed 10 years ago by nthiery

  • Description modified (diff)
Note: See TracTickets for help on using tickets.