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.

Sign up for GitHub

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

Revise == and isequal for ring elements #1854

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Revise == and isequal for ring elements #1854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a30f7c4
Default == and isequal for NCRingElem
fingolfin Oct 9, 2024
db82bbc
Replace some custom checks by check_parent
fingolfin Oct 9, 2024
4ef9fc7
Delete redundant isequal method
fingolfin Oct 9, 2024
fd7aa4e
Fix isequal for RationalFunctionFieldElem
fingolfin Oct 9, 2024
260d487
Strict parent checks in == methods
fingolfin Oct 9, 2024
0a069dc
Lax parent checks in isequal methods
fingolfin Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/ring_interface.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ function ==(f::ConstPoly{T}, g::ConstPoly{T}) where T <: RingElement
end

function isequal(f::ConstPoly{T}, g::ConstPoly{T}) where T <: RingElement
check_parent(f, g)
parent(f) == parent(g) || return false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got more generous (but it's only example code)

return isequal(f.c, g.c)
end

Expand Down
8 changes: 3 additions & 5 deletions src/AbsSeries.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::AbsPowerSeriesRingElem{T}, y::AbsPowerSeriesRingElem{T}) where T <: RingElement
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)

prec = min(precision(x), precision(y))
m1 = min(length(x), length(y))
Expand Down Expand Up @@ -523,9 +522,8 @@ power series are precisely the same, to the same precision, are they declared
equal by this function.
"""
function isequal(x::AbsPowerSeriesRingElem{T}, y::AbsPowerSeriesRingElem{T}) where T <: RingElement
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false

if precision(x) != precision(y) || length(x) != length(y)
return false
end
Expand Down
7 changes: 2 additions & 5 deletions src/Fraction.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::FracElem{T}, y::FracElem{T}) where {T <: RingElem}
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got stricter. An alternative would be to allow comparison of x and y with differing parents, but IMHO only if that means arithmetic comparison, i.e.: removing the parent check entirely and just proceeding to the following checks involving comparison of numerators and denominators and all. Returning false just because the parents are different is IMHO the worst of all ways to handle this, and a source of errors.

(To be clear, I prefer the error; the point I am trying to make here is that the current behavior is worse than either of the alternatives)


return (denominator(x, false) == denominator(y, false) &&
numerator(x, false) == numerator(y, false)) ||
Expand All @@ -437,9 +436,7 @@ inexact, e.g. power series. Only if the power series are precisely the same,
to the same precision, are they declared equal by this function.
"""
function isequal(x::FracElem{T}, y::FracElem{T}) where {T <: RingElem}
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false
return isequal(numerator(x, false)*denominator(y, false),
denominator(x, false)*numerator(y, false))
end
Expand Down
6 changes: 2 additions & 4 deletions src/Matrix.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,8 +1303,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::MatrixElem{T}, y::MatrixElem{T}) where {T <: NCRingElement}
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)
for i = 1:nrows(x)
for j = 1:ncols(x)
if x[i, j] != y[i, j]
Expand All @@ -1324,8 +1323,7 @@ series. Only if the power series are precisely the same, to the same precision,
are they declared equal by this function.
"""
function isequal(x::MatrixElem{T}, y::MatrixElem{T}) where {T <: NCRingElement}
b = check_parent(x, y, false)
!b && return false
parent(x) == parent(y) || return false
for i = 1:nrows(x)
for j = 1:ncols(x)
if !isequal(x[i, j], y[i, j])
Expand Down
16 changes: 6 additions & 10 deletions src/NCPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,13 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::NCPolyRingElem{T}, y::NCPolyRingElem{T}) where T <: NCRingElem
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)
if length(x) != length(y)
return false
else
for i = 1:length(x)
if coeff(x, i - 1) != coeff(y, i - 1)
return false
end
end
for i = 1:length(x)
if coeff(x, i - 1) != coeff(y, i - 1)
return false
end
end
return true
Expand All @@ -365,9 +363,7 @@ power series. Only if the power series are precisely the same, to the same
precision, are they declared equal by this function.
"""
function isequal(x::NCPolyRingElem{T}, y::NCPolyRingElem{T}) where T <: NCRingElem
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false
if length(x) != length(y)
return false
end
Expand Down
25 changes: 16 additions & 9 deletions src/NCRings.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,22 @@ end

*(x::NCRingElement, y::NCRingElem) = parent(y)(x)*y

function ==(x::NCRingElem, y::NCRingElem)
fl, u, v = try_promote(x, y)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's look into retaining this, and documenting this promotion system (and then perhaps we can also use it to simplify some other code which does ad-hoc stuff right now)

if fl
return u == v
else
return false
end
end

# == throws an error when inputs have different parent to help user
# find bugs in their code
function ==(a::NCRingElem, b::NCRingElem)
check_parent(a, b)
throw(NotImplementedError(:(==), x, y))
end

# isequal treats ring elements with differing parent as simply being not equal
# (instead of throwing an exception like == does) as it is used to compare set
# elements or keys in dictionaries, and there it seems at least plausible to
# allow mixed parents
function isequal(a::NCRingElem, b::NCRingElem)
return parent(a) == parent(b) && a == b
end


==(x::NCRingElem, y::NCRingElement) = x == parent(x)(y)

==(x::NCRingElement, y::NCRingElem) = parent(y)(x) == y
Expand Down
18 changes: 8 additions & 10 deletions src/Poly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,15 +839,14 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::PolyRingElem{T}, y::PolyRingElem{T}) where T <: RingElement
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)

if length(x) != length(y)
return false
else
for i = 1:length(x)
if coeff(x, i - 1) != coeff(y, i - 1)
return false
end
end
for i = 1:length(x)
if coeff(x, i - 1) != coeff(y, i - 1)
return false
end
end
return true
Expand All @@ -862,9 +861,8 @@ power series. Only if the power series are precisely the same, to the same
precision, are they declared equal by this function.
"""
function isequal(x::PolyRingElem{T}, y::PolyRingElem{T}) where T <: RingElement
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false

if length(x) != length(y)
return false
end
Expand Down
8 changes: 3 additions & 5 deletions src/RelSeries.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::RelPowerSeriesRingElem{T}, y::RelPowerSeriesRingElem{T}) where T <: RingElement
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)

xval = valuation(x)
xprec = precision(x)
Expand Down Expand Up @@ -762,9 +761,8 @@ power series are precisely the same, to the same precision, are they declared
equal by this function.
"""
function isequal(x::RelPowerSeriesRingElem{T}, y::RelPowerSeriesRingElem{T}) where T <: RingElement
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false

if precision(x) != precision(y) || pol_length(x) != pol_length(y) ||
valuation(x) != valuation(y)
return false
Expand Down
6 changes: 2 additions & 4 deletions src/Residue.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(a::ResElem{T}, b::ResElem{T}) where {T <: RingElement}
fl = check_parent(a, b, false)
!fl && return false
check_parent(a, b)
return data(a) == data(b)
end

Expand All @@ -257,8 +256,7 @@ Only if the power series are precisely the same, to the same precision, are
they declared equal by this function.
"""
function isequal(a::ResElem{T}, b::ResElem{T}) where {T <: RingElement}
fl = check_parent(a, b, false)
!fl && return false
parent(a) == parent(b) || return false
return isequal(data(a), data(b))
end

Expand Down
3 changes: 1 addition & 2 deletions src/ResidueField.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(a::ResFieldElem{T}, b::ResFieldElem{T}) where {T <: RingElement}
fl = check_parent(a, b, false)
!fl && return false
check_parent(a, b)
return data(a) == data(b)
end

Expand Down
4 changes: 0 additions & 4 deletions src/Rings.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
#
###############################################################################

function isequal(a::RingElem, b::RingElem)
return parent(a) == parent(b) && a == b
end

# Implement `isapprox` for ring elements via equality by default. On the one
# hand, we need isapprox methods to be able to conformance test series rings.
# On the other hand this is essentially the only sensible thing to do in
Expand Down
10 changes: 5 additions & 5 deletions src/algorithms/GenericFunctions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ end
Return `mod(f^e, m)` but possibly computed more efficiently.
"""
function powermod(a::T, n::Integer, m::T) where T <: RingElem
parent(a) == parent(m) || error("Incompatible parents")
check_parent(a, m)
if n > 1
return internal_powermod(a, n, m)
elseif n == 1
Expand Down Expand Up @@ -149,7 +149,7 @@ case `q` is set to the quotient, or `flag` is set to `false` and `q`
is set to `zero(f)`.
"""
function divides(a::T, b::T) where T <: RingElem
parent(a) == parent(b) || error("Incompatible parents")
check_parent(a, b)
if iszero(b)
return iszero(a), b
end
Expand All @@ -166,7 +166,7 @@ the cofactor after $f$ is divided by this power.
See also [`valuation`](@ref), which only returns the valuation.
"""
function remove(a::T, b::T) where T <: Union{RingElem, Number}
parent(a) == parent(b) || error("Incompatible parents")
check_parent(a, b)
if (iszero(b) || is_unit(b))
throw(ArgumentError("Second argument must be a non-zero non-unit"))
end
Expand Down Expand Up @@ -205,7 +205,7 @@ any other common divisor of $a$ and $b$ divides $g$.
way that if the return is a unit, that unit should be one.
"""
function gcd(a::T, b::T) where T <: RingElem
parent(a) == parent(b) || error("Incompatible parents")
check_parent(a, b)
while !iszero(b)
(a, b) = (b, mod(a, b))
end
Expand Down Expand Up @@ -287,7 +287,7 @@ Return a triple `d, s, t` such that $d = gcd(f, g)$ and $d = sf + tg$, with $s$
loosely reduced modulo $g/d$ and $t$ loosely reduced modulo $f/d$.
"""
function gcdx(a::T, b::T) where T <: RingElem
parent(a) == parent(b) || error("Incompatible parents")
check_parent(a, b)
R = parent(a)
if iszero(a)
if iszero(b)
Expand Down
2 changes: 1 addition & 1 deletion src/generic/FactoredFraction.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ function *(b::Integer, a::FactoredFracFieldElem{T}) where T <: RingElem
end

function *(b::FactoredFracFieldElem{T}, c::FactoredFracFieldElem{T}) where T <: RingElement
parent(b) == parent(c) || error("Incompatible rings")
check_parent(b, c)
input_is_good = _bases_are_coprime(b) && _bases_are_coprime(b)
z = FactoredFracFieldElem{T}(b.unit*c.unit, FactoredFracTerm{T}[], parent(b))
if iszero(z.unit)
Expand Down
3 changes: 1 addition & 2 deletions src/generic/FreeAssociativeAlgebra.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,7 @@ end
###############################################################################

function ==(a::FreeAssociativeAlgebraElem{T}, b::FreeAssociativeAlgebraElem{T}) where T
fl = check_parent(a, b, false)
!fl && return false
check_parent(a, b)
return a.length == b.length &&
view(a.exps, 1:a.length) == view(b.exps, 1:b.length) &&
view(a.coeffs, 1:a.length) == view(b.coeffs, 1:b.length)
Expand Down
2 changes: 1 addition & 1 deletion src/generic/LaurentMPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ end
###############################################################################

function ==(a::LaurentMPolyWrap, b::LaurentMPolyWrap)
check_parent(a, b, false) || return false
check_parent(a, b)
if a.mindegs == b.mindegs
return a.mpoly == b.mpoly
end
Expand Down
4 changes: 2 additions & 2 deletions src/generic/LaurentPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ function canonical_unit(p::LaurentPolyWrap)
end

function gcd(p::LaurentPolyWrap{T}, q::LaurentPolyWrap{T}) where T
parent(p) == parent(q) || error("Incompatible parents")
check_parent(p, q)
if iszero(p)
return divexact(q, canonical_unit(q))
elseif iszero(q)
Expand All @@ -249,7 +249,7 @@ function gcd(p::LaurentPolyWrap{T}, q::LaurentPolyWrap{T}) where T
end

function gcdx(a::LaurentPolyWrap{T}, b::LaurentPolyWrap{T}) where T
parent(a) == parent(b) || error("Incompatible parents")
check_parent(a, b)
R = parent(a)
if iszero(a)
if iszero(b)
Expand Down
7 changes: 2 additions & 5 deletions src/generic/LaurentSeries.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1037,8 +1037,7 @@ that power series to different precisions may still be arithmetically
equal to the minimum of the two precisions.
"""
function ==(x::LaurentSeriesElem{T}, y::LaurentSeriesElem{T}) where {T <: RingElement}
b = check_parent(x, y, false)
!b && return false
check_parent(x, y)
xval = valuation(x)
xprec = precision(x)
yval = valuation(y)
Expand Down Expand Up @@ -1094,9 +1093,7 @@ power series are precisely the same, to the same precision, are they declared
equal by this function.
"""
function isequal(x::LaurentSeriesElem{T}, y::LaurentSeriesElem{T}) where {T <: RingElement}
if parent(x) != parent(y)
return false
end
parent(x) == parent(y) || return false
if precision(x) != precision(y) || pol_length(x) != pol_length(y) ||
valuation(x) != valuation(y) || scale(x) != scale(y)
return false
Expand Down
3 changes: 1 addition & 2 deletions src/generic/MPoly.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2022,8 +2022,7 @@ end
###############################################################################

function ==(a::MPoly{T}, b::MPoly{T}) where {T <: RingElement}
fl = check_parent(a, b, false)
!fl && return false
check_parent(a, b)
if a.length != b.length
return false
end
Expand Down
4 changes: 2 additions & 2 deletions src/generic/PuiseuxSeries.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,7 @@ end
###############################################################################

function ==(a::PuiseuxSeriesElem{T}, b::PuiseuxSeriesElem{T}) where T <: RingElement
fl = check_parent(a, b, false)
!fl && return false
check_parent(a, b)
s = gcd(a.scale, b.scale)
zscale = div(a.scale*b.scale, s)
ainf = div(a.scale, s)
Expand All @@ -565,6 +564,7 @@ function ==(a::PuiseuxSeriesElem{T}, b::PuiseuxSeriesElem{T}) where T <: RingEle
end

function isequal(a::PuiseuxSeriesElem{T}, b::PuiseuxSeriesElem{T}) where T <: RingElement
parent(a) == parent(b) || return false
return a.scale == b.scale && isequal(a.data, b.data)
end

Expand Down
2 changes: 1 addition & 1 deletion src/generic/RationalFunctionField.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ end

function isequal(a::RationalFunctionFieldElem{T, U}, b::RationalFunctionFieldElem{T, U}) where {T <: FieldElement, U <: Union{PolyRingElem, MPolyRingElem}}
check_parent(a, b)
return data(a) == data(b)
return isequal(data(a), data(b))
end

###############################################################################
Expand Down
Loading
Loading