Fix IsIntegralRing to return false for rings that are euclidean b…

…ut not integral by changing `IsUniqueFactorizationRing` and `IsEuclideanRing` to not imply `IsIntegralRing` anymore (#5817)

* Bugfix and test for IsIntegralRing

This fixes an off-by-one error causing missing the case
when zero appears in the diagonal in the multiplication table.

Closes #3975.

* Remove incorrect text in IsUniqueFactorizationRing docs

Fields are in filter IsUniqueFactorizationRing since GAP 4.8

* IsUniqueFactorizationRing no longer implies IsIntegral

Hence IsEuclideanRing also does not anymore

---------

Co-authored-by: Alexander Konovalov <[email protected]>

lib/ring.gd

@@ -491,29 +491,15 @@ DeclareOperation( "ClosureRing", [ IsRing, IsObject ] );
 ##  <Filt Name="IsUniqueFactorizationRing" Arg='R' Type='Category'/>
 ##
 ##  <Description>
-##  A ring <A>R</A> is called a <E>unique factorization ring</E> if it is an
-##  integral ring (see&nbsp;<Ref Prop="IsIntegralRing"/>),
-##  and every nonzero element has a unique factorization into
+##  A ring <A>R</A> is called a <E>unique factorization ring</E> if
+##  every nonzero element has a unique factorization into
 ##  irreducible elements,
 ##  i.e., a  unique representation as product of irreducibles
 ##  (see <Ref Oper="IsIrreducibleRingElement"/>).
 ##  Unique in this context means unique up to permutations of the factors and
 ##  up to multiplication of the factors by units
 ##  (see&nbsp;<Ref Attr="Units"/>).
 ##  <P/>
-##  Mathematically, a field should therefore also be a unique factorization
-##  ring, since every nonzero element is a unit.
-##  In &GAP;, however,
-##  at least at present fields do not lie in the filter
-##  <Ref Filt="IsUniqueFactorizationRing"/>,
-##  since operations such as <Ref Oper="Factors"/>,
-##  <Ref Func="Gcd" Label="for (a ring and) several elements"/>,
-##  <Ref Oper="StandardAssociate"/> and so on do
-##  not apply to fields (the results would be trivial, and not
-##  especially useful) and methods which require their arguments to
-##  lie in <Ref Filt="IsUniqueFactorizationRing"/> expect these operations
-##  to work.
-##  <P/>
 ##  (Note that we cannot install a subset maintained method for this filter
 ##  since the factorization of an element needs not exist in a subring.
 ##  As an example, consider the subring <M>4 &NN; + 1</M> of the ring
@@ -545,7 +531,7 @@ DeclareCategory( "IsUniqueFactorizationRing", IsRing );
 ##  <Filt Name="IsEuclideanRing" Arg='R' Type='Category'/>
 ##
 ##  <Description>
-##  A ring <M>R</M> is called a Euclidean ring if it is an integral ring and
+##  A ring <M>R</M> is called a Euclidean ring if it is a non-trivial commutative ring and
 ##  there exists a function <M>\delta</M>, called the Euclidean degree, from
 ##  <M>R-\{0_R\}</M> into a well-ordered set (such as the nonnegative integers),
 ##  such that for every pair <M>r \in R</M> and <M>s \in  R-\{0_R\}</M> there
@@ -624,7 +610,7 @@ InstallSubsetMaintenance( IsIntegralRing,
 InstallTrueMethod( IsIntegralRing,
     IsRing and IsMagmaWithInversesIfNonzero and IsNonTrivial );
 InstallTrueMethod( IsIntegralRing,
-    IsUniqueFactorizationRing and IsNonTrivial );
+    IsRing and IsCyclotomicCollection and IsNonTrivial );
 
 
 #############################################################################

lib/ring.gi

@@ -702,7 +702,7 @@ InstallMethod( IsIntegralRing,
         zero := Zero( R );
         for i  in [1..Length(elms)]  do
             if elms[i] = zero then continue; fi;
-            for k  in [i+1..Length(elms)]  do
+            for k  in [i..Length(elms)]  do
                 if elms[k] = zero then continue; fi;
                 if elms[i] * elms[k] = zero then
                     return false;

tst/testinstall/ring.tst

@@ -0,0 +1,34 @@
+gap> START_TEST("ring.tst");
+
+#####################################################################
+#
+# Tests for IsIntegralRing
+#
+# Trivial ring
+gap> IsIntegralRing( SmallRing(1,1) );
+false
+
+# Non-commutative ring
+gap> IsIntegralRing( SmallRing(4,7) );
+false
+
+# Zero divisors on the diagonal
+gap> IsIntegralRing( SmallRing(4,3) );
+false
+
+# Zero divisors not on the diagonal
+gap> IsIntegralRing( Integers mod 6 );
+false
+
+# Integral rings
+gap> IsIntegralRing( GF(5) );
+true
+gap> IsIntegralRing( Integers );
+true
+gap> IsIntegralRing( Rationals );
+true
+gap> IsIntegralRing( CF(4) );
+true
+
+#
+gap> STOP_TEST( "ring.tst" );