gap-system / gap

Main development repository for GAP - Groups, Algorithms, Programming, a System for Computational Discrete Algebra
https://www.gap-system.org
GNU General Public License v2.0
816 stars 161 forks source link

IsMultiplicativeElementWithZero false when it should be true #1237

Closed laurentbartholdi closed 6 years ago

laurentbartholdi commented 7 years ago

MultiplicativeElementWithZero means that an element can be multiplied and zeroed; Zero(x) has the property that Zero(x)=Zero(y) and Zero(x)*y=Zero(x) for all x,y.

From this, I would expect IsMultiplicativeElementWithZero(1)=true; but it's false.

I ask this because I implemented Nillity for multiplicative objects with zero: "is there n>0 such that x^n=0". The correct filter seemed to be IsMultiplicativeElementWithZero.

fingolfin commented 6 years ago

This is not what IsMultiplicativeElementWithZero means. From its documentation:

This is the category of elements in a family which can be the operands of * (multiplication) and the operation MultiplicativeZero.

And there is no MultiplicativeZero for the integer 0:

gap> MultiplicativeZero(0);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
laurentbartholdi commented 6 years ago

I agree that there is no MultiplicativeZero for the integer 0; but now there are 2 bugs, not 1:

gap ?MultiplicativeZero MultiplicativeZero returns the multiplicative zero of the magma M which is the element z in M such that z m = m z = z for all m in M.

so MultiplicativeZero only applies to magmas, not elements.

gap> G := SymmetricGroup(3);; A := MagmaWithZeroAdjoined(G);; gap> MultiplicativeZero(G); MultiplicativeZero(A); MultiplicativeZero(A.1); fail <group with 0 adjoined elt: 0> Error, no method found! For debugging hints type ?Recovery from NoMethodFound

so far, so good.

gap> IsMultiplicativeElementWithZero(A.1); true

should return false, since there is no MultiplicativeZero for A.1

gap> MultiplicativeZero(Integers); ... infinite loop...

should return true, because there is an element of Integers acting as 0.

fingolfin commented 6 years ago

I meant MultiplicativeZeroOp, which applies to elements (no idea why they are split into two operations, unlike Zero and One). Which also fails.

So one could of course retroactively add all these things. But in the end, I am not sure this is achievable, since the integers play a very special role in GAP.

The whole zoo of IsMultiplicativeElementWithZero, IsAdditiveElementWithZero, etc. etc. in the end is possibly overdesigned and widely underused or incorrectly used.

Still, I agree that there is a deeper issue here. Either IsMagma should be dropped from the integers, or the missing features for the Integers added, or some documentation clarification should explicitly exclude the integers.

fingolfin commented 6 years ago

One fundamental problem here is that IsMultiplicativeElementWithZero is a filter, not a property. So it is intrinsic to the object. But the mathematical property in its core is tied to the magma we consider.

But e.g. the positive natural numbers with multiplication also form a magma, containing 1, and this magma contains no multiplicative zero. So in that sense, IsMultiplicativeElementWithZero(1) = false is correct. Of course if one considers it as an element of the integers, then it is wrong. The point is: mathematically, the question IsMultiplicativeElementWithZero(1) is not well-defined.

However, the correct (if possible unintuitive) view is to consider this as a purely technical thing (which filters are). Then the documentation of IsMultiplicativeElementWithZero actually says just how it is (albeit with a small mistake):

This is the category of elements in a family which can be the operands of * (multiplication) and the operation MultiplicativeZero.

(The mistake of course is that it should say MultiplicativeZeroOp, not MultiplicativeZero.

And, well, for the family containing 1, MultiplicativeZeroOp isn't implemented, hence the answer given by GAP, false, is correct.

fingolfin commented 6 years ago

So in your example, it should be MultiplicativeZeroOp(A.1) instead of MultiplicativeZero(A.1) -- although curiously, both work for me in the master branch -- turns out @james-d-mitchell added this in commit 43222917e18ea49780a41e42c0cbefdc030e1eed but did not update the documentation to mention this. (Perhaps we should just merge MultiplicativeZeroOp into MultiplicativeZero and retain the former only as a synonym for the latter, marked as obsolete?)

laurentbartholdi commented 6 years ago

I fear it's too common for Xxx and XxxOp to be two versions of the same thing, as in Orbit and OrbitOp. How about renaming MultiplicativeZeroOp into MultiplicativeZeroOfElement ? It seems, contrary to your suggestion to make one obsolete, that it's useful to have a command for elements and one for magmas.

fingolfin commented 6 years ago

Actually, Xxx and XxxOp never (or at least almost never, I don't claim to have checked the whole library and all packages) refers to the same thing. e.g. Orbit is a documented function, and does all kind of preprocessing and multi arg handling, while OrbitOp is an undocumented operation for which custom methods can be installed. Code using OrbitOp directly is usually suspicious (although of course there are some cases where it is the correct thing to do, but ideally should be restricted to the library, users should never have to deal with it)

This kind of split is quote common and almost always what is going on when one sees: a function or operation (e.g. First, SylowSubgroup, ...) paired with an operation.

But this is not the case with MultiplicativeZeroOp and MultiplicativeZero. Rather, for some unclear and confusing reason I can not fathom, the former is reserved for elements, the latter is reserved for magmas. This is in stark contrast with Zero and One, both of which are equally applicable to elements (e.g. ring elements) and domains (e.g. rings).

My suggestion hence aimed at making MultiplicativeZero more similar to Zero and One, as that is what I'd expected if I'd encounter it for the first time (i.e. what happened yesterday, when I blindly trusted the documentation and used MultiplicativeZero(0)). Apparently @james-d-mitchell agrees, at least that would explain why he installed that MultiplicativeZero method which, for elements, calls MultiplicativeZeroOp.

With my suggestion, you could write both MultiplicativeZero(M) and MultiplicativeZero(M.1). There would be no need anymore for MultiplicativeZeroOp (and no functionality would be lost). The only reason to keep MultiplicativeZeroOp at all is for backwards compatibility with existing code using it.

laurentbartholdi commented 6 years ago

OK, this makes sense!

Mathematically speaking, every element has a(t least one) zero, so I understand why designers wanted it to be a property of the magma. On the other hand, is most common use is for elements.

On Dec 20, 2017 14:11, "Max Horn" notifications@github.com wrote:

Actually, Xxx and XxxOp never (or at least almost never, I don't claim to have checked the whole library and all packages) refers to the same thing. e.g. Orbit is a documented function, and does all kind of preprocessing and multi arg handling, while OrbitOp is an undocumented operation for which custom methods can be installed. Code using OrbitOp directly is usually suspicious (although of course there are some cases where it is the correct thing to do, but ideally should be restricted to the library, users should never have to deal with it)

This kind of split is quote common and almost always what is going on when one sees: a function or operation (e.g. First, SylowSubgroup, ...) paired with an operation.

But this is not the case with MultiplicativeZeroOp and MultiplicativeZero. Rather, for some unclear and confusing reason I can not fathom, the former is reserved for elements, the latter is reserved for magmas. This is in stark contrast with Zero and One, both of which are equally applicable to elements (e.g. ring elements) and domains (e.g. rings).

My suggestion hence aimed at making MultiplicativeZero more similar to Zero and One, as that is what I'd expected if I'd encounter it for the first time (i.e. what happened yesterday, when I blindly trusted the documentation and used MultiplicativeZero(0)). Apparently @james-d-mitchell https://github.com/james-d-mitchell agrees, at least that would explain why he installed that MultiplicativeZero method which, for elements, calls MultiplicativeZeroOp.

With my suggestion, you could write both MultiplicativeZero(M) and MultiplicativeZero(M.1). There would be no need anymore for MultiplicativeZeroOp (and no functionality would be lost). The only reason to keep MultiplicativeZeroOp at all is for backwards compatibility with existing code using it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gap-system/gap/issues/1237#issuecomment-353059267, or mute the thread https://github.com/notifications/unsubscribe-auth/ACIIUDtHuLZ2wesk_cv_6d7pQ4BJ0-LJks5tCQd7gaJpZM4M8v-j .

james-d-mitchell commented 6 years ago

One thing that no one mentioned is MultiplicativeNeutralElement, which returns the identity of a magma if it has one. This can sometimes be the One of any element in the magma, and some times not. For example, in S := Semigroup(Transformation([2, 2, 3, 4])); the One of S.1 is the identity transformation, but this does not belong to S so One(S) returns fail but MultiplicativeNeutralElement returns Transformation([2, 2, 3, 4]) since this element acts as the identity in S. BTW also MultiplicativeZero(S) returns Transformation([2, 2, 3, 4]) I believe all of this is the correct behaviour.

I think MultiplicativeZero should behave in the same way as MultiplicativeNeutralElement does (and I think it currently does), and that Zero should behave in the same way as One (it does not currently). It appears that MultiplicativeZeroOp was intended to behave like One since Zero does not apply to multiplicative elements at all but rather to additive elements.

So, I think that Zero is the thing that needs to be changed here, and MultiplicativeZeroOp should be removed. If Zero applied to elements in IsMultiplicativeElementWithZero and to IsAdditiveElementWithZero, then this could be done.

laurentbartholdi commented 6 years ago

I think we're opening a can of worms.

Zero(x) is clearly defined as the additive neutral element of the object x. However, MultiplicativeZero and MultiplicativeOne are not uniquely-defined, so what should GAP return?

In James' example, One(S.1) could just as well be S.1^0 as S.1, according to the documentation:

gap> ?One; ?1 OneImmutable, OneMutable, and OneSameMutability return the multiplicative neutral element of the multiplicative element obj.

Already this is misleading because there is no "the" multiplicative neutral element.

I believe we all have the reflex that One(S.1) should return the "one" of the "natural thing in which S.1 sits", namely the whole family of multiplicative elements. One(S) should, in my opinion, return the "one" of S, namely S.1.

AdditiveZero is well-defined, as long as addition is invertible. MultiplicativeZero should return any zero. For semigroups, it's even worse: there should be "LeftMultiplicativeZero" and "RightMultiplicativeZero"...

I don't think we'll get this right before christmas.

On Thu, Dec 21, 2017 at 10:56 AM, James Mitchell notifications@github.com wrote:

So, I'm not sure if I have too much to add, maybe one of you already mentioned this, but there are situations where MultiplicativeZero(M) should return one thing, and MultiplicativeZero(M.1) something else (namely fail in the first instance and something that is not fail in the other case). For example, if M.1 belongs to a category of elements that have a zero (i.e. belongs to IsMultiplicativeElementWithZero) but the magma M does not contain this zero element. This is my understanding anyway.

Another thing that no one mentioned is MultiplicativeNeutralElement, which returns the identity of a magma if it has one. This can sometimes be the One of any element in the magma, and some times not. For example, in S := Semigroup(Transformation([2, 2, 3, 4])); the One of S.1 is the identity transformation, but this does not belong to S so One(S) returns fail but MultiplicativeNeutralElement returns Transformation([2, 2, 3, 4]) since this element acts as the identity in S. BTW also MultiplicativeZero(S) returns Transformation([2, 2, 3, 4]) I believe all of this is the correct behaviour.

I think MultiplicativeZero should behave in the same way as MultiplicativeNeutralElement does (and I think it currently does), and that Zero should behave in the same way as One (it does not currently). It appears that MultiplicativeZeroOp was intended to behave like One since Zero does not apply to multiplicative elements at all but rather to additive elements.

So, I think that Zero is the thing that needs to be changed here, and MultiplicativeZeroOp should be removed. If Zero applied to elements in IsMultiplicativeElementWithZero and to IsAdditiveElementWithZero, then this could be done.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gap-system/gap/issues/1237#issuecomment-353308799, or mute the thread https://github.com/notifications/unsubscribe-auth/ACIIUH66CWkdrGBEjytF8jOq2zHHVLNNks5tCitFgaJpZM4M8v-j .

-- Laurent Bartholdi laurent.bartholdigmailcom École Normale Supérieure: Téléphone +33 1 44322060 Bureau T5, Toits du DMA, 45 Rue d'Ulm, 75005 Paris G.-A. Universität zu Göttingen: Phone: +49 551 39 7826 Office 202, Mathematisches Institut, Bunsenstrasse 3-5, D-37073 Göttingen.

fingolfin commented 6 years ago

I have a hard time following the last comments as they seem to omit key information: Basically, it makes no sense to say something like "MultiplicativeZero is not uniquely defined"; beause depending on how I interpret, this statement is either false, true, or undefined.

In order to make sense out of what you say, I have to guess whether you were talking about applying these operations to algebraic structures, or to elements. The former is usually sane and well-defined; the latter is were almost all the problems are (to use the slogan @mohamed-barakat likes: the problem is that GAP is based, at its technical core, on elements, no algebraic structure or categories; which is OK in a naive view of mathematics, but of course makes no sense algebraically).

So: MultiplicativeZero(M) for a magma M is uniquely defined, due to property "z m = m z = z for all m". Any two zeros must thus be equal. Of course one could also define left/right multiplicative zeros, and then those are not unique (if the magma is not commutative), but as soon as a magma has both a left and a right zero, the two must be equal, and we again get uniqueness.

In contrast, MultiplicativeZeroOp(x) for an element x is not uniquely defined in an obvious way, and hence tricky. We attempt to get out of that by cheating, by assuming that there is an implicit "maximal" algebraic structure M (described by the "family" of x), and then defining MultiplicativeZeroOp(x) as equal to MultiplicativeZero(M). The problem with that is that one might then make the silent assumption that if x \in N holds for any "substructure" N of M, then MultiplicativeZero(M) = MultiplicativeZeroOp(x) = MultiplicativeZero(N) holds -- but this is clearly only true if x in N, and that in turn is usually WRONG.

Variants of the same problems affect Zero and One (and their various variants), and they would also affect MultiplicativeNeutralElement if that was defined for elements (but as far as I can tell, it isn't, which avoids this pit fall). And it goes far beyond that, e.g. IsPrime(2) -- is it true or not? Well, depends on your ring. It's true in the integers, its false in the Rationals, and there are infinitely many rings in between where it goes either way.

The "clean" solution, if one could travel back in time, would perhaps be one of these two:

  1. forbid almost all operations which only take elements but not a domain (so e.g. no IsPrime(x), just IsPrime(R,x));
  2. force elements to live in a single well-defined algebraic structure, and require some form of coercion / casting / conversion to re-interpret it as elements of another algebraic structure.

Both of these approaches have theirs pros and cons, compared to each other and the current state. The current state is often very convenient, as long as things "work as expected", but as you can see in this discussion, nasty pitfalls lurk. The alternatives are perhaps cleaner but can also be very annoying. The second one is kind of what Magma does, but I think it only works if you take it to full extreme (e.g. 1 would have to be either a rational or an integer; not both; the integers shouldn't be a subset of the rationals), and I think Magma (and Sage, and other systems) actually try a compromise... which in the end runs into the same problems again, anyway (e.g. in Magma, you can "prove" 0 = 1 via a clever chain of coercions).

I therefore disagree about Laurent's believe of us all sharing a "reflex" for what One(S.1) should do -- my reflex to that (before learning GAP) would have been "huh, what is that?", and these day is "I wish we never had allowed that" -- BTW, this term I again taught students GAP, and I observed that none expected One(x) to be acceptable, for x an element... so I think while it may be convenient, it is not at all natural, and as this mess here shows, it is outright dangerous.

fingolfin commented 6 years ago

@james-d-mitchell wrote:

I think MultiplicativeZero should behave in the same way as MultiplicativeNeutralElement does (and I think it currently does), and that Zero should behave in the same way as One (it does not currently). It appears that MultiplicativeZeroOp was intended to behave like One since Zero does not apply to multiplicative elements at all but rather to additive elements.

I don't understand this paragraph, mostly because (a) I am not sure we are talking about arguments which are elements; algebraic structures; or either; and (b) I couldn't figure out what " Zero should behave in the same way as One" should possibly mean.

james-d-mitchell commented 6 years ago

@fingolfin in my comment above I meant that MultiplicativeZero and MultiplicativeNeutralElement should be applied to algebraic structures only, and that One and Zero should be applicable to both elements and algebraic structures.

What I meant about One and Zero behaving the same is that it would be great if Zero could also be applied to multiplicative elements (not only additive, as is the case at present), with the following behaviour:

I believe that if Zero is replaced with One, and MultiplicativeZero is replaced with MultiplicativeNeutralElement in these three bullet points (and the definition of multiplicative zero is replaced with that of multiplicative identity), then this describes how GAP currently behaves, and I am ok with this.

IsMonoid is something that confuses many users too. IsMonoid really means IsInTheCategoryOfMonoids rather than IsThisGAPObjectMathematicallyAMonoid. The former means that every element has a One, the latter means (or would mean if it existed) that there is a MultiplicativeNeutralElement. In the Semigroups package, we have a property called IsMonoidAsSemigroup which checks if the MultiplicativeNeutralElement exists or not.