Opened 6 years ago

Closed 6 years ago

#16405 closed enhancement (fixed)

Cartesian product of rings

Reported by: nthiery Owned by:
Priority: major Milestone: sage-6.3
Component: categories Keywords: cartesian product
Cc: sage-combinat, ncohen, vdelecroix, dimpase Merged in:
Authors: Nicolas M. Thiéry Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: eced39e (Commits) Commit: eced39e483d11838ad826694a7fd29e26c3cdf64
Dependencies: Stopgaps:

Description

This ticket lifts up cartesian product features to the categories recently created in #10963, where they belong. Thanks to this and to the definition of CartesianProducts? for the Distributive axiom, we have now:

sage: Fields().CartesianProducts()
Join of Category of rings
    and Category of Cartesian products of distributive magmas and additive magmas
    and Category of Cartesian products of semigroups 
    and Category of Cartesian products of unital magmas
    and Category of Cartesian products of additive inverse additive unital additive magmas 
    and Category of Cartesian products of additive commutative additive magmas 
    and Category of Cartesian products of additive semigroups

And this works smoothly for all variants of rings (semirings, ...).

This fixes a piece of : #15425.

Change History (11)

comment:1 Changed 6 years ago by nthiery

  • Branch set to u/nthiery/cartesian_product_of_rings

comment:2 Changed 6 years ago by nthiery

  • Commit set to 8b547db0ecdae108a3fa94b69010b579edb0d149
  • Status changed from new to needs_review

New commits:

8b547db16405: cartesian product of rings and generalization of many cartesian product features

comment:3 Changed 6 years ago by ncohen

Is it normal that we see

    and Category of Cartesian products of additive inverse additive unital additive magmas 
    and Category of Cartesian products of additive commutative additive magmas 
    and Category of Cartesian products of additive semigroups

instead of "Additive Group" ?

Nathann

comment:4 follow-up: Changed 6 years ago by ncohen

Helloooooooooooo !!

Some comments/questions:

  • What are sub/neg doing in AdditiveMagma().AdditiveUnital() ?
  • consistancy -> consistency
  • class nesting is cool and everything but you have to scroll non-stop with stuff like emacs.... Do you have a trick to know in which sub/sub/sub/sub class you are writing your code ?
  • You implement the neg function for cartesian product of AdditiveInverse? Magma, but don't you also need to say somewhere what exactly is the "1" element of a cartesian product of UnitalMagma? ?
  • You do things like :
    Magmas().Unital().Inverse()
    

It feels like the syntax should be Magmas().Unital().Inverse(), which would imply Unital. What do you think ?

  • maybe I misread but you seem to implement __invert__ in Magmas().Unital().CartesianProduct(). Shouldn't it be several lines above, in Magmas().Unital().Inverse().CartesianProduct() ?

Nathann

comment:5 Changed 6 years ago by ncohen

  • Status changed from needs_review to needs_info

comment:6 Changed 6 years ago by git

  • Commit changed from 8b547db0ecdae108a3fa94b69010b579edb0d149 to 27f6c9aa0206d77349f4759062b85ce4fc8f737a

Branch pushed to git repo; I updated commit sha1. New commits:

27f6c9a16405: better handling and documentation of partially defined __neg__ and __invert__

comment:7 in reply to: ↑ 4 Changed 6 years ago by nthiery

Salut Nathann,

Thanks for the quick review!

Replying to ncohen:

Some comments/questions:

  • What are sub/neg doing in AdditiveMagma().AdditiveUnital() ?
  • maybe I misread but you seem to implement __invert__ in Magmas().Unital().CartesianProduct(). Shouldn't it be several lines above, in Magmas().Unital().Inverse().CartesianProduct() ?

The notion of inverse (resp. negation) can be defined as soon as there is a unit, thought some elements might not have inverses, or have non unique inverses. So the method __invert__ (resp. __neg__) makes sense, even if it's only partially defined.

I just pushed a fix that better handles those partially defined inverse / negation for cartesian products, and clarifies the documentation.

  • consistancy -> consistency

Fixed!

  • class nesting is cool and everything but you have to scroll non-stop with stuff like emacs.... Do you have a trick to know in which sub/sub/sub/sub class you are writing your code ?

Alas no. Code folding is really a life saver here, but I still need to dig through the emacs packages to find one that implements such folding with a reasonable interface.

  • You implement the neg function for cartesian product of AdditiveInverse? Magma, but don't you also need to say somewhere what exactly is the "1" element of a cartesian product of UnitalMagma? ?

I think this was there. In any cases, right now we have:

Magmas.Unital.CartesianProducts.ParentMethods.one
Magmas.Unital.CartesianProducts.ElementMethods.__invert__

AdditiveMagmas.AdditiveUnital.CartesianProducts.ParentMethods.zero
AdditiveMagmas.AdditiveUnital.CartesianProducts.ElementMethods._neg_
  • You do things like :
    Magmas().Unital().Inverse()
    

It feels like the syntax should be Magmas().Unital().Inverse(), which would imply Unital. What do you think ?

You mean Magmas().Inverse() as a shorthand for Magmas().Unital().Inverse()? I wondered about this, but so far preferred to only define the Inverse axiom if there exists a unit.

One thing is that there exist a notion of inverse semigroups where an element is only requested to have a "local" inverse (technically it has to be invertible in some submonoid whose unit need not be a unit for the whole semigroup). So I'd rather leave the room free for future extensions.

Cheers,

Nicolas


New commits:

27f6c9a16405: better handling and documentation of partially defined __neg__ and __invert__

New commits:

27f6c9a16405: better handling and documentation of partially defined __neg__ and __invert__

comment:8 Changed 6 years ago by git

  • Commit changed from 27f6c9aa0206d77349f4759062b85ce4fc8f737a to eced39e483d11838ad826694a7fd29e26c3cdf64

Branch pushed to git repo; I updated commit sha1. New commits:

eced39e16405: fixed trivial doctest failure and ReST indentation

comment:9 follow-up: Changed 6 years ago by ncohen

  • Cc dimpase added
  • Status changed from needs_info to positive_review

Ahahahaah. Reviewing this code is *MUCH* easier with emacs' which-function-mode. Night and day !

I agree with it, knowing that I am not all-knowledeagle on category stuff. But I do pay attention, I do try, and I may eventually learn.

Thanks for this patch !

Nathann

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

Replying to ncohen:

Ahahahaah. Reviewing this code is *MUCH* easier with emacs' which-function-mode. Night and day !

Pretty cool tool indeed! Thanks for finding it! (for the curious emacs users: if you enable this minor mode by customizing which-function-mode, the name of the class / function the cursor is in is printed in the status bar; works better with emacs 24).

I agree with it, knowing that I am not all-knowledeagle on category stuff. But I do pay attention, I do try, and I may eventually learn.

You are definitely learning it quick :-)

Thanks for this patch !

Thanks for the review!

comment:11 Changed 6 years ago by vbraun

  • Branch changed from u/nthiery/cartesian_product_of_rings to eced39e483d11838ad826694a7fd29e26c3cdf64
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.