Tests and fixes for factoring related methods (#1596)

- is_squarefree: zero is never square free, units always are
- factor and factor_squarefree throw a uniform error if argument is zero
- add a bunch of tests to verify the fixes above

src/HeckeMiscInteger.jl

@@ -231,12 +231,8 @@ end
 Returns true if $n$ is squarefree, false otherwise.
 """
 function is_squarefree(n::Union{Int,ZZRingElem})
-    if iszero(n)
-        error("Argument must be non-zero")
-    end
-    if isone(abs(n))
-        return true
-    end
+    iszero(n) && return false
+    is_unit(n) && return true
     e, b = is_power(n)
     if e > 1
         return false

src/HeckeMiscPoly.jl

@@ -501,9 +501,8 @@ end
 function is_squarefree(f::PolyRingElem{<:FieldElement})
   R = coefficient_ring(f)
 
-  if iszero(f) || degree(f) == 0
-    return true
-  end
+  iszero(f) && return false
+  degree(f) == 0 && return true
 
   if !is_monic(f)
     g = divexact(f, leading_coefficient(f))
@@ -520,9 +519,8 @@ function is_squarefree(f::PolyRingElem{<:FieldElement})
 end
 
 function is_squarefree(f::PolyRingElem{<:RingElement})
-  if iszero(f)
-    return true
-  end
+  iszero(f) && return false
+  degree(f) == 0 && return is_squarefree(leading_coefficient(f))
   fac = factor_squarefree(f)
   return all(e <= 1 for (_, e) in fac)
 end

src/flint/fmpq_mpoly.jl

@@ -516,6 +516,7 @@ function (::Type{Fac{QQMPolyRingElem}})(fac::fmpq_mpoly_factor, preserve_input::
 end
 
 function factor(a::QQMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fmpq_mpoly_factor(R)
    ok = ccall((:fmpq_mpoly_factor, libflint), Cint,
@@ -526,6 +527,7 @@ function factor(a::QQMPolyRingElem)
 end
 
 function factor_squarefree(a::QQMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fmpq_mpoly_factor(R)
    ok = ccall((:fmpq_mpoly_factor_squarefree, libflint), Cint,

src/flint/fmpq_poly.jl

@@ -692,6 +692,7 @@ for (factor_fn, factor_fn_inner, flint_fn) in
    eval(quote
 
       function $factor_fn(x::QQPolyRingElem)
+         iszero(x) && throw(ArgumentError("Argument must be non-zero"))
          res, z = $factor_fn_inner(x)
          return Fac(parent(x)(z), res)
       end

src/flint/fmpz.jl

@@ -1484,9 +1484,7 @@ function _factor(a::ZZRingElem)
 end
 
 function factor(a::T) where T <: Union{Int, UInt}
-   if iszero(a)
-      throw(ArgumentError("Argument is not non-zero"))
-   end
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    u = sign(a)
    a = u < 0 ? -a : a
    F = n_factor()
@@ -1585,9 +1583,7 @@ julia> factor(12)
 ```
 """
 function factor(a::ZZRingElem)
-   if iszero(a)
-      throw(ArgumentError("Argument is not non-zero"))
-   end
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    fac, z = _factor(a)
    return Fac(z, fac)
 end

src/flint/fmpz_mod_mpoly.jl

@@ -532,6 +532,7 @@ function (::Type{Fac{($etype)}})(fac::($ftype), preserve_input::Bool = true)
 end
 
 function factor(a::($etype))
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = ($ftype)(R)
    ok = ccall((:fmpz_mod_mpoly_factor, libflint), Cint,
@@ -542,6 +543,7 @@ function factor(a::($etype))
 end
 
 function factor_squarefree(a::($etype))
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = ($ftype)(R)
    ok = ccall((:fmpz_mod_mpoly_factor_squarefree, libflint), Cint,

src/flint/fmpz_mod_poly.jl

@@ -761,6 +761,7 @@ end
 ################################################################################
 
 function factor(x::ZZModPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   !is_probable_prime(modulus(x)) && error("Modulus not prime in factor")
   fac = _factor(x)
   return Fac(parent(x)(leading_coefficient(x)), fac)
@@ -786,6 +787,7 @@ function _factor(x::ZZModPolyRingElem)
 end
 
 function factor_squarefree(x::ZZModPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   !is_probable_prime(modulus(x)) && error("Modulus not prime in factor_squarefree")
   fac = _factor_squarefree(x)
   return Fac(parent(x)(leading_coefficient(x)), fac)

src/flint/fmpz_mpoly.jl

@@ -487,6 +487,7 @@ function (::Type{Fac{ZZMPolyRingElem}})(fac::fmpz_mpoly_factor, preserve_input::
 end
 
 function factor(a::ZZMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fmpz_mpoly_factor(R)
    ok = ccall((:fmpz_mpoly_factor, libflint), Cint,
@@ -497,6 +498,7 @@ function factor(a::ZZMPolyRingElem)
 end
 
 function factor_squarefree(a::ZZMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fmpz_mpoly_factor(R)
    ok = ccall((:fmpz_mpoly_factor_squarefree, libflint), Cint,

src/flint/fmpz_poly.jl

@@ -697,6 +697,7 @@ for (factor_fn, factor_fn_inner, flint_fn) in
   eval(quote
 
     function $factor_fn(x::ZZPolyRingElem)
+      iszero(x) && throw(ArgumentError("Argument must be non-zero"))
       fac, z = $factor_fn_inner(x)
       ffac = factor(z)
 

src/flint/fq_default_mpoly.jl

@@ -267,10 +267,12 @@ function _convert_fac(a::FqMPolyRing, b::Fac)
 end
 
 function factor(a::FqMPolyRingElem)
+    iszero(a) && throw(ArgumentError("Argument must be non-zero"))
     return _convert_fac(parent(a), factor(a.data))
 end
 
 function factor_squarefree(a::FqMPolyRingElem)
+    iszero(a) && throw(ArgumentError("Argument must be non-zero"))
     return _convert_fac(parent(a), factor_squarefree(a.data))
 end
 

src/flint/fq_default_poly.jl

@@ -646,6 +646,7 @@ function length(x::fq_default_poly_factor)
 end   
 
 function factor(x::FqPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
    fac, z = _factor(x)
    return Fac(parent(x)(z), fac)
 end
@@ -671,6 +672,7 @@ function _factor(x::FqPolyRingElem)
 end
 
 function factor_squarefree(x::FqPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   # _factor_squareefree does weird things if the polynomial is not monic
   return Fac(parent(x)(leading_coefficient(x)),
 	      _factor_squarefree(divexact(x, leading_coefficient(x))))

src/flint/fq_nmod_mpoly.jl

@@ -445,6 +445,7 @@ function (::Type{Fac{fqPolyRepMPolyRingElem}})(fac::fq_nmod_mpoly_factor, preser
 end
 
 function factor(a::fqPolyRepMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fq_nmod_mpoly_factor(R)
    ok = ccall((:fq_nmod_mpoly_factor, libflint), Cint,
@@ -455,6 +456,7 @@ function factor(a::fqPolyRepMPolyRingElem)
 end
 
 function factor_squarefree(a::fqPolyRepMPolyRingElem)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = fq_nmod_mpoly_factor(R)
    ok = ccall((:fq_nmod_mpoly_factor_squarefree, libflint), Cint,

src/flint/fq_nmod_poly.jl

@@ -637,6 +637,7 @@ end
 ################################################################################
 
 function factor(x::fqPolyRepPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
    res, z = _factor(x)
    return Fac(parent(x)(z), res)
 end
@@ -662,6 +663,7 @@ function _factor(x::fqPolyRepPolyRingElem)
 end
 
 function factor_squarefree(x::fqPolyRepPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   # _factor_squareefree does weird things if the polynomial is not monic
   return Fac(parent(x)(leading_coefficient(x)),
 	     _factor_squarefree(divexact(x, leading_coefficient(x))))

src/flint/fq_poly.jl

@@ -635,6 +635,7 @@ end
 ################################################################################
 
 function factor(x::FqPolyRepPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
    fac, z = _factor(x)
    return Fac(parent(x)(z), fac)
 end
@@ -660,6 +661,7 @@ function _factor(x::FqPolyRepPolyRingElem)
 end
 
 function factor_squarefree(x::FqPolyRepPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   # _factor_squareefree does weird things if the polynomial is not monic
   return Fac(parent(x)(leading_coefficient(x)),
 	      _factor_squarefree(divexact(x, leading_coefficient(x))))

src/flint/gfp_fmpz_poly.jl

@@ -342,6 +342,7 @@ end
 ################################################################################
 
 function factor(x::FpPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   fac = _factor(x)
   return Fac(parent(x)(leading_coefficient(x)), fac)
 end
@@ -367,6 +368,7 @@ function _factor(x::FpPolyRingElem)
 end  
 
 function factor_squarefree(x::FpPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   fac = _factor_squarefree(x)
   return Fac(parent(x)(leading_coefficient(x)), fac)
 end

src/flint/gfp_poly.jl

@@ -391,6 +391,7 @@ end
 ################################################################################
 
 function factor(x::fpPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   fac, z = _factor(x)
   return Fac(parent(x)(z), fac)
 end
@@ -411,6 +412,7 @@ function _factor(x::fpPolyRingElem)
 end
 
 function factor_squarefree(x::fpPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   return Fac(parent(x)(leading_coefficient(x)), _factor_squarefree(x))
 end
 

src/flint/nmod_mpoly.jl

@@ -490,6 +490,7 @@ function (::Type{Fac{($etype)}})(fac::($ftype), preserve_input::Bool = true)
 end
 
 function factor(a::($etype))
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = ($ftype)(R)
    ok = ccall((:nmod_mpoly_factor, libflint), Cint,
@@ -500,6 +501,7 @@ function factor(a::($etype))
 end
 
 function factor_squarefree(a::($etype))
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    R = parent(a)
    fac = ($ftype)(R)
    ok = ccall((:nmod_mpoly_factor_squarefree, libflint), Cint,

src/flint/nmod_poly.jl

@@ -720,6 +720,7 @@ end
 ################################################################################
 
 function factor(x::zzModPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   fac, z = _factor(x)
   return Fac(parent(x)(z), fac)
 end
@@ -741,6 +742,7 @@ function _factor(x::zzModPolyRingElem)
 end
 
 function factor_squarefree(x::zzModPolyRingElem)
+  iszero(x) && throw(ArgumentError("Argument must be non-zero"))
   !is_prime(modulus(x)) && error("Modulus not prime in factor_squarefree")
   return Fac(parent(x)(leading_coefficient(x)), _factor_squarefree(x))
 end

src/gaussiannumbers/ZZi.jl

@@ -803,6 +803,7 @@ function _sum_of_squares(p::ZZRingElem)
 end
 
 function factor(a::fmpzi)
+   iszero(a) && throw(ArgumentError("Argument must be non-zero"))
    f = Fac{fmpzi}()
    g = gcd(a.x, a.y)
    f.unit = divexact(a, g)   # throw if a=0

test/flint/fmpq_mpoly-test.jl

@@ -525,6 +525,9 @@ end
 @testset "QQMPolyRingElem.factor" begin
    R, (x, y, z) = polynomial_ring(FlintQQ, ["x", "y", "z"])
 
+   @test_throws ArgumentError factor(R(0))
+   @test_throws ArgumentError factor_squarefree(R(0))
+
    function check_factor(a, esum)
       f = factor(a)
       @test a == unit(f) * prod([p^e for (p, e) in f])

test/flint/fmpq_poly-test.jl

@@ -488,6 +488,9 @@ end
 @testset "QQPolyRingElem.factor" begin
    S, y = polynomial_ring(QQ, "y")
 
+   @test_throws ArgumentError factor(S(0))
+   @test_throws ArgumentError factor_squarefree(S(0))
+
    f = (2y + 1)^10*(5*y^3 + 1)^100*(-QQFieldElem(1,5))
 
    fac = factor(f)
@@ -499,13 +502,22 @@ end
 
    @test f == unit(fac) * prod([ p^e for (p, e) in fac])
 
+   @test_throws ArgumentError factor(S(0))
+   @test_throws ArgumentError factor_squarefree(S(0))
+
+   @test !is_irreducible(S(0))
+   @test !is_irreducible(S(1))
    @test !is_irreducible(S(2))
    @test is_irreducible(y^4 + 1)
    @test is_irreducible(y + 1)
    @test !is_irreducible(S(4))
    @test is_irreducible(2y + 2)
    @test !is_irreducible(y^2)
 
+   @test !is_squarefree(S(0))
+   @test is_squarefree(S(1))
+   @test is_squarefree(S(2))
+   @test is_squarefree(S(4))
    @test is_squarefree(7*y^2 + 2)
    @test is_squarefree(2*y)
    @test is_squarefree(4*y)

test/flint/fmpz_mod_poly-test.jl

@@ -502,17 +502,25 @@ end
 
 @testset "ZZModPolyRingElem.is_squarefree" begin
    R = residue_ring(ZZ, 123456789012345678949)
-   S, x = polynomial_ring(R, "x")
+   Rx, x = polynomial_ring(R, "x")
 
    f = x^2 + 2x + 1
 
    @test is_squarefree(f) == false
+
+   @test !is_squarefree(Rx(0))
+   @test is_squarefree(Rx(1))
+   @test is_squarefree(Rx(2))
+   @test is_squarefree(Rx(4))
 end
 
 @testset "ZZModPolyRingElem.factor" begin
    R = residue_ring(ZZ, 123456789012345678949)
    S, x = polynomial_ring(R, "x")
 
+   @test_throws ArgumentError factor(S(0))
+   @test_throws ArgumentError factor_squarefree(S(0))
+
    f = 3*(x^2 + 2x + 1)
    g = x^3 + 3x + 1
 

test/flint/fmpz_mpoly-test.jl

@@ -505,6 +505,9 @@ end
 @testset "ZZMPolyRingElem.factor" begin
    R, (x, y, z) = polynomial_ring(FlintZZ, ["x", "y", "z"])
 
+   @test_throws ArgumentError factor(R(0))
+   @test_throws ArgumentError factor_squarefree(R(0))
+
    function check_factor(a, esum)
       f = factor(a)
       @test is_unit(unit(f))