Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cartesian product of rings #16405

Closed
nthiery opened this issue May 27, 2014 · 13 comments
Closed

Cartesian product of rings #16405

nthiery opened this issue May 27, 2014 · 13 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented May 27, 2014

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.

CC: @sagetrac-sage-combinat @nathanncohen @videlec @dimpase

Component: categories

Keywords: cartesian product

Author: Nicolas M. Thiéry

Branch/Commit: eced39e

Reviewer: Nathann Cohen

Issue created by migration from https://trac.sagemath.org/ticket/16405

@nthiery nthiery added this to the sage-6.3 milestone May 27, 2014
@nthiery
Copy link
Contributor Author

nthiery commented May 27, 2014

Branch: u/nthiery/cartesian_product_of_rings

@nthiery
Copy link
Contributor Author

nthiery commented May 27, 2014

New commits:

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

@nthiery
Copy link
Contributor Author

nthiery commented May 27, 2014

Commit: 8b547db

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 27, 2014

comment:3

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

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 27, 2014

comment:4

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 28, 2014

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 28, 2014

Changed commit from 8b547db to 27f6c9a

@nthiery
Copy link
Contributor Author

nthiery commented May 28, 2014

comment:7

Salut Nathann,

Thanks for the quick review!

Replying to @nathanncohen:

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__

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2014

Changed commit from 27f6c9a to eced39e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 2, 2014

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

eced39e16405: fixed trivial doctest failure and ReST indentation

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jun 2, 2014

comment:9

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

@nthiery
Copy link
Contributor Author

nthiery commented Jun 2, 2014

comment:10

Replying to @nathanncohen:

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!

@vbraun
Copy link
Member

vbraun commented Jun 2, 2014

Changed branch from u/nthiery/cartesian_product_of_rings to eced39e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants