Opened 10 years ago

Closed 10 years ago

#8881 closed enhancement (fixed)

Functorial constructions in categories

Reported by: hivert Owned by: nthiery
Priority: critical Milestone: sage-4.4.4
Component: categories Keywords: Functorial constructions
Cc: sage-combinat Merged in: sage-4.4.4.alpha0
Authors: Nicolas M. Thiéry Reviewers: Florent Hivert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by hivert)

This patch refactors completely the framework for (covariant) functorial constructions, improves the existing ones, and adds new ones.

Improved constructions:

  • Cartesian products: generalization to non modules_with_basis, monoid operations, ...

New constructions:

Miscellaneous category fixes and improvements:

  • is_subcategory now handles properly join categories (courtesy of Florent Hivert)
  • For a field K, Modules(K) returns VectorSpaces?(K)
  • As a temporary workaround, Parent._an_element_ calls the _an_element_ method provided by the categories, if available
  • Adds a method Category.or_subcategory
  • Cleans up _repr_ by extracting a _repr_object_names method
  • Cleans up _latex_ by extracting a _short_name method

Depend on #9104

Attachments (2)

trac_8881-functorial_constructions-nt.patch (204.4 KB) - added by nthiery 10 years ago.
Renames TensorFunctor? to TensorProductFunctor?
diff (1.0 KB) - added by nthiery 10 years ago.
Diff to previous version of the patch

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by hivert

  • Authors set to Nicolas M. Thiéry
  • Reviewers set to Florent Hivert
  • Status changed from new to needs_review

The patch is on review on sage-combinat-queue and should be ready soon. Please Nicolas, can you fold my last review patch if you are ok with it and upload the bunch here. I'll probably put a positive review after a last re-reading (and at least 5 folded review patches) :-)

Florent

comment:2 follow-up: Changed 10 years ago by nthiery

  • Cc sage-combinat added
  • Description modified (diff)
  • Keywords constructions added; construction removed

comment:3 in reply to: ↑ 2 ; follow-up: Changed 10 years ago by hivert

  • Priority changed from major to critical

Replying to nthiery:

One last remark, otherwise I'm ready to give positive review: there is a slight naming inconsistency:

  • CartesianProductFunctor, CartesianProducts, CartesianProductsCategory, cartesian_product
  • TensorFunctor, TensorProducts, TensorProductsCategory, tensor

I'm just not 100% sure that we don't want TensorProductFunctor and tensor_product...

Any arguments ?

By the way I put Priority=critical because they are at least a dozen of patch on sage-combinat which depend on this one...

Changed 10 years ago by nthiery

Diff to previous version of the patch

comment:4 in reply to: ↑ 3 Changed 10 years ago by nthiery

Replying to hivert:

One last remark, otherwise I'm ready to give positive review: there is a slight naming inconsistency:

  • CartesianProductFunctor, CartesianProducts, CartesianProductsCategory, cartesian_product
  • TensorFunctor, TensorProducts, TensorProductsCategory, tensor

I'm just not 100% sure that we don't want TensorProductFunctor and tensor_product...

Any arguments ?

Yup, we had discussed this with David Roe during the review of the category patches. And we had agreed that tensor([A,B,C]) was shorter and more practical than tensor_product([A,B,C]), yet clear and unambiguous. On the other hand cartesian([A,B,C]) wasn't that clear. So we decided to favor here practicality over consistency.

On the other hand, I fixed TensorFunctor? to TensorProductFunctor? in the updated patch.

comment:5 follow-up: Changed 10 years ago by hivert

  • Status changed from needs_review to positive_review

Excellent ! Positive review unless Massena find a new bug (which is VeryUnlikely(TM)).

comment:6 in reply to: ↑ 5 Changed 10 years ago by nthiery

Replying to hivert:

Excellent ! Positive review unless Massena find a new bug (which is VeryUnlikely(TM)).

Thanks for all your work on this!

comment:7 follow-up: Changed 10 years ago by was

  • Status changed from positive_review to needs_work

This causes doctest failures, including the following.

sage -t  sage/categories/sets_cat.py
**********************************************************************
File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/devel/sage-main/sage/categories/sets_cat.py", line 624:
    sage: A = Monoids().example().algebra(QQ); A
Exception raised:
    Traceback (most recent call last):
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_16[4]>", line 1, in <module>
        A = Monoids().example().algebra(QQ); A###line 624:
    sage: A = Monoids().example().algebra(QQ); A
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/categories/sets_cat.py", line 637, in algebra
        return CombinatorialFreeModule(base_ring, self, category = category.Algebras(base_ring))
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/misc/classcall_metaclass.py", line 256, in __call__
        return cls.__classcall_private__(cls, *args, **options)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/combinat/free_module.py", line 850, in __classcall_private__
        return super(CombinatorialFreeModule, cls).__classcall__(cls, *args, **keywords)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/misc/cachefunc.py", line 115, in __call__
        w = self.f(*args, **kwds)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/structure/unique_representation.py", line 447, in __classcall__
        instance = type.__call__(cls, *args, **options)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/combinat/free_module.py", line 892, in __init__
        element_constructor = self._element_constructor_)
      File "parent.pyx", line 457, in sage.structure.parent.Parent.__init__ (sage/structure/parent.c:3851)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/categories/magmas.py", line 115, in __init_extra__
        if (self.product != self.product_from_element_class_mul) and hasattr(self, "element_class") and hasattr(self.element_class, "_mul_parent"):
      File "element.pyx", line 860, in sage.structure.element.Element.__richcmp__ (sage/structure/element.c:7061)
      File "element.pyx", line 801, in sage.structure.element.Element._richcmp (sage/structure/element.c:6441)
      File "coerce.pyx", line 907, in sage.structure.coerce.CoercionModel_cache_maps.canonical_coercion (sage/structure/coerce.c:8537)
      File "sage_object.pyx", line 101, in sage.structure.sage_object.SageObject.__repr__ (sage/structure/sage_object.c:1370)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/categories/homset.py", line 302, in _repr_
        self._domain, self._codomain, self.__category)
      File "sage_object.pyx", line 101, in sage.structure.sage_object.SageObject.__repr__ (sage/structure/sage_object.c:1370)
      File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/local/lib/python/site-packages/sage/combinat/free_module.py", line 1163, in _repr_
        return self._name + " over %s"%self.base_ring()
      File "parent.pyx", line 676, in sage.structure.parent.Parent.__getattr__ (sage/structure/parent.c:5239)
      File "parent.pyx", line 263, in sage.structure.parent.getattr_from_other_class (sage/structure/parent.c:2753)
      File "parent.pyx", line 171, in sage.structure.parent.raise_attribute_error (sage/structure/parent.c:2625)
    AttributeError: 'CombinatorialFreeModule_with_category' object has no attribute '_name'
**********************************************************************
File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/devel/sage-main/sage/categories/sets_cat.py", line 626:
    sage: A.category()
Expected:
    Category of monoid algebras over Rational Field
Got:
    Category of set algebras over Rational Field
**********************************************************************
1 items had failures:
   2 of   8 in __main__.example_16
***Test Failed*** 2 failures.
For whitespace errors, see the file /scratch/wstein/sage//tmp/.doctest_sets_cat.py

comment:8 in reply to: ↑ 7 ; follow-up: Changed 10 years ago by hivert

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Replying to was:

This causes doctest failures, including the following.

> 
> sage -t  sage/categories/sets_cat.py
> **********************************************************************
> File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/devel/sage-main/sage/categories/sets_cat.py", line 624:
>     sage: A = Monoids().example().algebra(QQ); A
> Exception raised:
. [...]
>     AttributeError: 'CombinatorialFreeModule_with_category' object has no attribute '_name'
> **********************************************************************
> File "/mnt/usb1/scratch/wstein/build/release/4.4.3/sage-4.4.3.alpha2/devel/sage-main/sage/categories/sets_cat.py", line 626:
>     sage: A.category()
> Expected:
>     Category of monoid algebras over Rational Field
> Got:
>     Category of set algebras over Rational Field
> **********************************************************************

This bug is fixed in #9104 but it does appear on 4.4.2 so I didn't think about a dependency. Sorry for this.

comment:9 Changed 10 years ago by hivert

  • Status changed from needs_review to positive_review

Note: I put back positive review though I had no chance to test on sage-4.4.3.alpha2...

comment:10 in reply to: ↑ 8 Changed 10 years ago by hivert

Replying to hivert:

This bug is fixed in #9104 but it does appear on 4.4.2 so I didn't think about a dependency. Sorry for this.

Actually it did (see the note there), but I completely forgot about it. My mistake.

comment:11 Changed 10 years ago by mhansen

  • Merged in set to sage-4.4.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.