Skip to content

Commit

Permalink
Introduce FlintInt and FlintRat helpers
Browse files Browse the repository at this point in the history
There are a bunch of FLINT function that come in multiple varieties.
E.g. fmpz_add, fmpz_add_ui, fmpz_add_si all take one fmpz and one
additional argument of type fmpz / UInt / Int, respectively.

So naturally we define different `+` and `add!` methods in Julia using
them. But which method to invoke when adding a `ZZRingElem` and a Julia
integer of a type different from `UInt` and `Int`, such as e.g. `UInt32`
? In that case we currently always convert that into a `ZZRingElem`. But
that's inefficient, it would be be better to convert that value to a
`UInt` or `Int` value and then use the optimized `fmpz_add_ui/_si`
method.

Of course this can be done, but getting it right is tedious, repetitive
and it is easy to overlook a case.

This is where `FlintInt` comes in: this is defined as `Union{Int,
ZZRingElem}`. We then provide constructors which convert any `Integer`
to a `FlintInt` in an optimal way (at least for a bunch of built-in
integer types).

This then can be used to write optimal dispatch for Integer types
like this:

    add!(x::ZRingElem, y::ZRingElem) = ...
    add!(x::ZRingElem, y::Int) = ...
    add!(x::ZRingElem, y::UInt) = ...
    # fallback code
    add!(x::ZRingElem, y::Integer) = add!(x, FlintInt(y))

It also works for types that accept ZZRingElem and Int, but not UInt:

    add!(x::ZPolyRingElem, y::ZPolyRingElem) = ...
    add!(x::ZPolyRingElem, y::ZRingElem) = ...
    add!(x::ZPolyRingElem, y::Int) = ...
    # fallback code
    add!(x::ZPolyRingElem, y::Integer) = add!(x, FlintInt(y))

Of course I might have missed optimal conversion for some `Integer`
subtype. But then we can fix this by simply adding another `FlintInt`
constructor in a single central place.

Finally, the new type `FlintRat` plays a similar role for FLINT
functions which also take a `QQFieldElem`. While `RationalUnion` is a
counterpart to `IntegerUnion`.
  • Loading branch information
fingolfin committed Sep 26, 2024
1 parent a47d0f6 commit b8c8bfe
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 67 deletions.
73 changes: 73 additions & 0 deletions src/flint/FlintTypes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,8 @@ mutable struct ZZMPolyRingElem <: MPolyRingElem{ZZRingElem}
z.parent = ctx
return z
end

ZZMPolyRingElem(ctx::ZZMPolyRing, a::Integer) = ZZMPolyRingElem(ctx, FlintInt(a))
end

function _fmpz_mpoly_clear_fn(a::ZZMPolyRingElem)
Expand Down Expand Up @@ -6453,8 +6455,79 @@ end
#
################################################################################

"""
IntegerUnion = Union{Integer, ZZRingElem}
The `IntegerUnion` type union allows convenient and compact declaration
of methods that accept both Julia and Nemo integers.
"""
const IntegerUnion = Union{Integer, ZZRingElem}

"""
RationalUnion = Union{Integer, ZZRingElem, Rational, QQFieldElem}
The `RationalUnion` type union allows convenient and compact declaration
of methods that accept both Julia and Nemo integers or rationals.
"""
const RationalUnion = Union{Integer, ZZRingElem, Rational, QQFieldElem}

"""
FlintInt = Union{Int, ZZRingElem}
The `FlintInt` type union is an internal helper for cleanly and compactly
implementing efficient dispatch for arithmetics that involve native Nemo
objects plus a Julia integer.
Many internal arithmetics functions in FLINT have optimize variants for
the case when one operand is an `ZZRingElem` or an `Int` (sometimes also
`UInt` is supported). E.g. there are special methods for adding
one of these to a `ZZRingPolyElem`.
In order to handling adding an arbitrary Julia integer to a
`ZZRingPolyElem`, further dispatch is needed. The easiest is to
provide a method
+(a::ZZRingPolyElem, b::Integer) = a + ZZ(b)
However this is inefficient when `b` is e.g. an `UInt16`. So we could
do this (at least on a 64 bit machine):
+(a::ZZRingPolyElem, b::Integer) = a + ZZ(b)
+(a::ZZRingPolyElem, b::{Int64,Int32,Int16,Int8,UInt32,UInt16,UInt8}) = a + Int(b)
Doing this repeatedly is cumbersome and error prone. This can be avoided by
using `FlintInt`, which allows us to write
+(a::ZZRingPolyElem, b::Integer) = a + FlintInt(b)
to get optimal dispatch.
This also works for Nemo types that also have special handlers for `UInt`,
as their method for `b::UInt` takes precedence over the fallback method.
"""
const FlintInt = Union{Int, ZZRingElem}

FlintInt(x::ZZRingElem) = x
FlintInt(x::Integer) = ZZRingElem(x)::ZZRingElem
if Int === Int64
FlintInt(x::Union{Int64,Int32,Int16,Int8,UInt32,UInt16,UInt8}) = Int(x)
else
FlintInt(x::Union{Int32,Int16,Int8,UInt16,UInt8}) = Int(x)

Check warning on line 6515 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L6515

Added line #L6515 was not covered by tests
end

"""
FlintRat = Union{FlintInt, QQFieldElem}
The `FlintRat` type union is an internal helper similar to [`FlintInt`](@ref)
but also accepting `QQFieldElem` and `Rational{<:Integer}` arguments.
"""
const FlintRat = Union{FlintInt, QQFieldElem}

FlintRat(x::QQFieldElem) = x
FlintRat(x::Rational{<:Integer}) = QQFieldElem(x)::QQFieldElem
FlintRat(x::Integer) = FlintInt(x)

Check warning on line 6528 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L6526-L6528

Added lines #L6526 - L6528 were not covered by tests


const ZmodNFmpzPolyRing = Union{ZZModPolyRing, FpPolyRing}

const Zmodn_poly = Union{zzModPolyRingElem, fpPolyRingElem}
Expand Down
41 changes: 10 additions & 31 deletions src/flint/fmpq_mpoly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -364,35 +364,35 @@ for (jT, cN, cT) in ((QQFieldElem, :fmpq, Ref{QQFieldElem}), (ZZRingElem, :fmpz,
end
end

+(a::QQMPolyRingElem, b::Integer) = a + ZZRingElem(b)
+(a::QQMPolyRingElem, b::Integer) = a + FlintInt(b)

+(a::Integer, b::QQMPolyRingElem) = b + a

-(a::QQMPolyRingElem, b::Integer) = a - ZZRingElem(b)
-(a::QQMPolyRingElem, b::Integer) = a - FlintInt(b)

-(a::Integer, b::QQMPolyRingElem) = -(b - a)
-(a::Integer, b::QQMPolyRingElem) = neg!(b - a)

+(a::QQMPolyRingElem, b::Rational{<:Integer}) = a + QQFieldElem(b)

+(a::Rational{<:Integer}, b::QQMPolyRingElem) = b + a

-(a::QQMPolyRingElem, b::Rational{<:Integer}) = a - QQFieldElem(b)

-(a::Rational{<:Integer}, b::QQMPolyRingElem) = -(b - a)
-(a::Rational{<:Integer}, b::QQMPolyRingElem) = neg!(b - a)

*(a::QQMPolyRingElem, b::Integer) = a * ZZRingElem(b)
*(a::QQMPolyRingElem, b::Integer) = a * FlintInt(b)

*(a::Integer, b::QQMPolyRingElem) = b * a

*(a::QQMPolyRingElem, b::Rational{<:Integer}) = a * QQFieldElem(b)

*(a::Rational{<:Integer}, b::QQMPolyRingElem) = b * a

divexact(a::QQMPolyRingElem, b::Integer; check::Bool=true) = divexact(a, ZZRingElem(b); check=check)
divexact(a::QQMPolyRingElem, b::Integer; check::Bool=true) = divexact(a, FlintInt(b); check=check)

divexact(a::QQMPolyRingElem, b::Rational{<:Integer}; check::Bool=true) = divexact(a, QQFieldElem(b); check=check)

//(a::QQMPolyRingElem, b::Integer) = //(a, ZZRingElem(b))
//(a::QQMPolyRingElem, b::Integer) = //(a, FlintInt(b))

//(a::QQMPolyRingElem, b::Rational{<:Integer}) = //(a, QQFieldElem(b))

Expand Down Expand Up @@ -573,7 +573,7 @@ end

==(a::Int, b::QQMPolyRingElem) = b == a

==(a::QQMPolyRingElem, b::Integer) = a == ZZRingElem(b)
==(a::QQMPolyRingElem, b::Integer) = a == FlintInt(b)

==(a::Integer, b::QQMPolyRingElem) = b == a

Expand Down Expand Up @@ -1181,33 +1181,12 @@ function (R::QQMPolyRing)()
return z
end

function (R::QQMPolyRing)(b::QQFieldElem)
function (R::QQMPolyRing)(b::RationalUnion)
z = QQMPolyRingElem(R, b)
return z
end

function (R::QQMPolyRing)(b::ZZRingElem)
z = QQMPolyRingElem(R, b)
return z
end

function (R::QQMPolyRing)(b::Int)
z = QQMPolyRingElem(R, b)
return z
end

function (R::QQMPolyRing)(b::UInt)
z = QQMPolyRingElem(R, b)
return z
end

function (R::QQMPolyRing)(b::Integer)
return R(ZZRingElem(b))
end

function (R::QQMPolyRing)(b::Rational{<:Integer})
return R(QQFieldElem(b))
end
QQMPolyRingElem(ctx::QQMPolyRing, a::RationalUnion) = QQMPolyRingElem(ctx, FlintRat(a))

function (R::QQMPolyRing)(a::QQMPolyRingElem)
parent(a) != R && error("Unable to coerce polynomial")
Expand Down
17 changes: 1 addition & 16 deletions src/flint/fmpz_mpoly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1061,26 +1061,11 @@ function (R::ZZMPolyRing)()
return z
end

function (R::ZZMPolyRing)(b::ZZRingElem)
function (R::ZZMPolyRing)(b::IntegerUnion)
z = ZZMPolyRingElem(R, b)
return z
end

function (R::ZZMPolyRing)(b::Int)
z = ZZMPolyRingElem(R, b)
return z
end

function (R::ZZMPolyRing)(b::UInt)
z = ZZMPolyRingElem(R, b)
return z
end

function (R::ZZMPolyRing)(b::Integer)
return R(ZZRingElem(b))
end


function (R::ZZMPolyRing)(a::ZZMPolyRingElem)
parent(a) != R && error("Unable to coerce polynomial")
return a
Expand Down
28 changes: 8 additions & 20 deletions src/flint/fmpz_poly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -225,17 +225,17 @@ end

*(x::ZZPolyRingElem, y::ZZRingElem) = y*x

+(x::Integer, y::ZZPolyRingElem) = y + ZZRingElem(x)
+(x::Integer, y::ZZPolyRingElem) = y + FlintInt(x)

Check warning on line 228 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L228

Added line #L228 was not covered by tests

-(x::Integer, y::ZZPolyRingElem) = ZZRingElem(x) - y
-(x::Integer, y::ZZPolyRingElem) = FlintInt(x) - y

Check warning on line 230 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L230

Added line #L230 was not covered by tests

*(x::Integer, y::ZZPolyRingElem) = ZZRingElem(x)*y
*(x::Integer, y::ZZPolyRingElem) = FlintInt(x)*y

Check warning on line 232 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L232

Added line #L232 was not covered by tests

+(x::ZZPolyRingElem, y::Integer) = x + ZZRingElem(y)
+(x::ZZPolyRingElem, y::Integer) = x + FlintInt(y)

Check warning on line 234 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L234

Added line #L234 was not covered by tests

-(x::ZZPolyRingElem, y::Integer) = x - ZZRingElem(y)
-(x::ZZPolyRingElem, y::Integer) = x - FlintInt(y)

Check warning on line 236 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L236

Added line #L236 was not covered by tests

*(x::ZZPolyRingElem, y::Integer) = ZZRingElem(y)*x
*(x::ZZPolyRingElem, y::Integer) = FlintInt(y)*x

Check warning on line 238 in src/flint/fmpz_poly.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/fmpz_poly.jl#L238

Added line #L238 was not covered by tests

###############################################################################
#
Expand Down Expand Up @@ -942,20 +942,8 @@ function (a::ZZPolyRing)()
return z
end

function (a::ZZPolyRing)(b::Int)
z = ZZPolyRingElem(b)
z.parent = a
return z
end

function (a::ZZPolyRing)(b::Integer)
z = ZZPolyRingElem(ZZRingElem(b))
z.parent = a
return z
end

function (a::ZZPolyRing)(b::ZZRingElem)
z = ZZPolyRingElem(b)
function (a::ZZPolyRing)(b::IntegerUnion)
z = ZZPolyRingElem(FlintInt(b))
z.parent = a
return z
end
Expand Down

0 comments on commit b8c8bfe

Please sign in to comment.