Skip to content

Commit

Permalink
fix sign+abs, avoid more ambiguities
Browse files Browse the repository at this point in the history
  • Loading branch information
benlorenz committed Jul 7, 2023
1 parent 817a11e commit b650ddc
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 11 deletions.
15 changes: 9 additions & 6 deletions src/integers.jl
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
# One arguments constructors used by convert:
# specialized Int64 constructors handled by Cxx side
(::Type{<:Integer})(int::BigInt) = new_integer_from_bigint(int)

Check warning on line 3 in src/integers.jl

View check run for this annotation

Codecov / codecov/patch

src/integers.jl#L3

Added line #L3 was not covered by tests
(::Type{<:Integer})(int::Number) = Integer(BigInt(int))
# to avoid ambiguities:
(::Type{<:Integer})(rat::Base.Rational) = Integer(BigInt(rat))
(::Type{<:Integer})(flt::BigFloat) = Integer(BigInt(flt))

Check warning on line 7 in src/integers.jl

View check run for this annotation

Codecov / codecov/patch

src/integers.jl#L6-L7

Added lines #L6 - L7 were not covered by tests
Integer(int::BigInt) = new_integer_from_bigint(int)
Integer(int::Number) = Integer(BigInt(int))
# to avoid ambiguities:
Integer(rat::Base.Rational) = Integer(BigInt(rat))
Integer(flt::BigFloat) = Integer(BigInt(flt))

# we need thie to make the fallbacks like Base.one work
IntegerAllocated(int::Union{BigInt,Base.Rational,BigFloat,<:Number}) = Integer(int)

import Base: ==, <, <=
# These are operations we delegate to gmp
for op in (:(==), :(<), :(<=))
Expand Down Expand Up @@ -43,15 +44,17 @@ for T in [:Int8, :Int16, :Int32, :UInt8, :UInt16, :UInt32]
@eval Base.$T(int::Integer) = $T(Int64(int))
end

(::Type{<:Rational})(int::Integer) = new_rational_from_integer(int)

Check warning on line 47 in src/integers.jl

View check run for this annotation

Codecov / codecov/patch

src/integers.jl#L47

Added line #L47 was not covered by tests
Rational(int::Integer) = new_rational_from_integer(int)
(::Type{T})(int::Integer) where {T<:Number} = convert(T, BigInt(int))
(::Type{T})(int::Integer) where {T<:AbstractFloat} = convert(T, Float64(int))
(::Type{T})(int::Integer) where T <: Number = convert(T, BigInt(int))
(::Type{T})(int::Integer) where T <: AbstractFloat = convert(T, Float64(int))

Check warning on line 50 in src/integers.jl

View check run for this annotation

Codecov / codecov/patch

src/integers.jl#L49-L50

Added lines #L49 - L50 were not covered by tests
# to avoid ambiguity
Float64(int::Integer) = Float64(CxxWrap.CxxRef(int))
BigFloat(int::Integer) = BigFloat(BigInt(int))
Base.float(int::Integer) = Float64(int)

# no-copy converts
(::Type{<:Integer})(int::Integer) = int
Integer(int::Integer) = int

Check warning on line 58 in src/integers.jl

View check run for this annotation

Codecov / codecov/patch

src/integers.jl#L58

Added line #L58 was not covered by tests
Base.Integer(int::Integer) = int

Expand Down
10 changes: 9 additions & 1 deletion src/oscarnumber.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ function Base.promote_rule(::Type{<:OscarNumber},
return OscarNumber
end

OscarNumber(a::Union{Base.Integer, Base.Rational{<:Base.Integer}}) = OscarNumber(Rational(a))
(::Type{<:OscarNumber})(a::Union{Base.Integer, Base.Rational{<:Base.Integer}}) = OscarNumber(Rational(a))

Check warning on line 6 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L6

Added line #L6 was not covered by tests
# this needs to be separate to avoid ambiguities
OscarNumber(a::Union{Base.Integer, Base.Rational{<:Base.Integer}}) = OscarNumber(Rational(a))
(::Type{<:OscarNumber})(a::Integer) = OscarNumber(Rational(a))

Check warning on line 9 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L9

Added line #L9 was not covered by tests
OscarNumber(a::Integer) = OscarNumber(Rational(a))
(::Type{<:OscarNumber})(a::Rational) = OscarNumber(CxxWrap.ConstCxxRef(a))

Check warning on line 11 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L11

Added line #L11 was not covered by tests
OscarNumber(a::Rational) = OscarNumber(CxxWrap.ConstCxxRef(a))

Base.zero(::Type{<:OscarNumber}) = OscarNumber(0)
Expand Down Expand Up @@ -301,11 +304,16 @@ function register_julia_element(e, p, t::Type)
end

(::Type{T})(on::OscarNumber) where T<:Number = convert(T, unwrap(on))
(::Type{<:Integer})(on::OscarNumber) = convert(Integer, unwrap(on))
(::Type{<:Rational})(on::OscarNumber) = convert(Rational, unwrap(on))

Check warning on line 308 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L307-L308

Added lines #L307 - L308 were not covered by tests
Integer(on::OscarNumber) = convert(Integer, unwrap(on))
Rational(on::OscarNumber) = convert(Rational, unwrap(on))

# we don't support conversion for concrete types inside the OscarNumber here
(::Type{<:QuadraticExtension{<:Rational}})(on::OscarNumber) = QuadraticExtension{Rational}(Rational(on))
QuadraticExtension{<:Rational}(on::OscarNumber) = QuadraticExtension{Rational}(Rational(on))

Check warning on line 314 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L313-L314

Added lines #L313 - L314 were not covered by tests
QuadraticExtension{Rational}(on::OscarNumber) = QuadraticExtension{Rational}(Rational(on))
(::Type{<:OscarNumber})(qe::QuadraticExtension) = OscarNumber(Rational(qe))
OscarNumber(qe::QuadraticExtension) = OscarNumber(Rational(qe))

Check warning on line 317 in src/oscarnumber.jl

View check run for this annotation

Codecov / codecov/patch

src/oscarnumber.jl#L316-L317

Added lines #L316 - L317 were not covered by tests

Base.float(on::OscarNumber) = float(unwrap(on))
8 changes: 8 additions & 0 deletions src/quadraticextension.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ QuadraticExtension{T}(a::Number, b::Number, r::Number) where T<:qe_suppT =
QuadraticExtension{T}(convert(T, a), convert(T, b), convert(T, r))

QuadraticExtension{T}(a::Number) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)
(::Type{<:QuadraticExtension{T}})(a::Number) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)

QuadraticExtension(x...) = QuadraticExtension{Rational}(x...)
(::Type{<:QuadraticExtension})(x...) = QuadraticExtension{Rational}(x...)

Check warning on line 10 in src/quadraticextension.jl

View check run for this annotation

Codecov / codecov/patch

src/quadraticextension.jl#L10

Added line #L10 was not covered by tests

# needed to avoid ambiguities
QuadraticExtension{T}(a::Integer) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)
(::Type{<:QuadraticExtension{T}})(a::Integer) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)

Check warning on line 14 in src/quadraticextension.jl

View check run for this annotation

Codecov / codecov/patch

src/quadraticextension.jl#L14

Added line #L14 was not covered by tests
QuadraticExtension(x::Integer) = QuadraticExtension{Rational}(x)
QuadraticExtension{T}(a::Rational) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)
(::Type{<:QuadraticExtension{T}})(a::Rational) where T<:qe_suppT = QuadraticExtension{T}(a, 0, 0)

Check warning on line 17 in src/quadraticextension.jl

View check run for this annotation

Codecov / codecov/patch

src/quadraticextension.jl#L17

Added line #L17 was not covered by tests
QuadraticExtension(a::Rational) = QuadraticExtension{Rational}(a, 0, 0)

Base.zero(::Type{<:QuadraticExtension{T}}) where T<:qe_suppT = QuadraticExtension{T}(0)
Expand Down Expand Up @@ -39,6 +43,8 @@ Base.:/(x::QuadraticExtension{T}, y::QuadraticExtension{T}) where T<:qe_suppT =

# no-copy convert
convert(::Type{<:QuadraticExtension{T}}, qe::QuadraticExtension{T}) where T<:qe_suppT = qe
(::Type{<:QuadraticExtension{T}})(qe::QuadraticExtension{T}) where T<:qe_suppT = qe
(QuadraticExtension{T})(qe::QuadraticExtension{T}) where T<:qe_suppT = qe

Check warning on line 47 in src/quadraticextension.jl

View check run for this annotation

Codecov / codecov/patch

src/quadraticextension.jl#L47

Added line #L47 was not covered by tests

function _qe_to_rational(::Type{T}, qe::QuadraticExtension) where T<:Number
!iszero(_b(qe)) && !iszero(_r(qe)) && throw(DomainError("Given QuadraticExtension not trivial."))
Expand All @@ -52,6 +58,8 @@ Base.promote_rule(::Type{<:QuadraticExtension{Rational}}, ::Type{<:AbstractFloat
(::Type{T})(qe::QuadraticExtension) where {T<:AbstractFloat} = convert(T, Float64(qe))

# avoid ambiguities
(::Type{<:Rational})(qe::QuadraticExtension) = _qe_to_rational(Rational,qe)
(::Type{<:Integer})(qe::QuadraticExtension) = _qe_to_rational(Integer,qe)

Check warning on line 62 in src/quadraticextension.jl

View check run for this annotation

Codecov / codecov/patch

src/quadraticextension.jl#L61-L62

Added lines #L61 - L62 were not covered by tests
Rational(qe::QuadraticExtension) = _qe_to_rational(Rational,qe)
Integer(qe::QuadraticExtension) = _qe_to_rational(Integer,qe)
(::Type{T})(qe::QuadraticExtension) where {T<:Base.Integer} = _qe_to_rational(T,qe)
11 changes: 10 additions & 1 deletion src/rationals.jl
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
(::Type{<:Rational})(num::Int64, den::Int64) =

Check warning on line 1 in src/rationals.jl

View check run for this annotation

Codecov / codecov/patch

src/rationals.jl#L1

Added line #L1 was not covered by tests
rational_si_si(convert(CxxLong, num), convert(CxxLong, den))
Rational(num::Int64, den::Int64) =
rational_si_si(convert(CxxLong, num), convert(CxxLong, den))

function Rational(num::T, den::S) where {T<:Base.Integer, S<:Base.Integer}
function (::Type{<:Rational})(num::T, den::S) where {T<:Base.Integer, S<:Base.Integer}
R = promote_type(promote_type(T, Int64), promote_type(S, Int64))
R == Int64 && return Rational(convert(Int64, num), convert(Int64, den))
return Rational(Integer(convert(BigInt, num)), Integer(convert(BigInt, den)))
end

(::Type{<:Rational})(x::Base.Rational) = Rational(numerator(x), denominator(x))

Check warning on line 12 in src/rationals.jl

View check run for this annotation

Codecov / codecov/patch

src/rationals.jl#L12

Added line #L12 was not covered by tests
Rational(x::Base.Rational) = Rational(numerator(x), denominator(x))

@inline function Rational(x::Base.Rational{BigInt})
GC.@preserve x new_rational_from_baserational(pointer_from_objref(numerator(x)), pointer_from_objref(denominator(x)))
end

(::Type{<:Rational})(int::Base.Integer) = Rational(int, one(int))
(::Type{<:Rational})(x::Number) = Rational(Base.Rational(x))

Check warning on line 20 in src/rationals.jl

View check run for this annotation

Codecov / codecov/patch

src/rationals.jl#L20

Added line #L20 was not covered by tests
Rational(int::Base.Integer) = Rational(int, one(int))
Rational(x::Number) = Rational(Base.Rational(x))

Expand Down Expand Up @@ -51,6 +58,7 @@ end
Base.Rational(rat::Rational) = Base.Rational{BigInt}(rat)
Base.big(rat::Rational) = Base.Rational{BigInt}(rat)

(::Type{<:Integer})(rat::Rational) = new_integer_from_rational(rat)

Check warning on line 61 in src/rationals.jl

View check run for this annotation

Codecov / codecov/patch

src/rationals.jl#L61

Added line #L61 was not covered by tests
Integer(rat::Rational) = new_integer_from_rational(rat)
(::Type{T})(rat::Rational) where T<:Number = convert(T, big(rat))
(::Type{T})(rat::Rational) where T<:AbstractFloat = convert(T, Float64(rat))
Expand All @@ -59,6 +67,7 @@ Base.float(rat::Rational) = Float64(rat)
Float64(rat::Rational) = Float64(CxxWrap.CxxRef(rat))

# no-copy convert
(::Type{<:Rational})(rat::Rational) = rat
Rational(rat::Rational) = rat

Check warning on line 71 in src/rationals.jl

View check run for this annotation

Codecov / codecov/patch

src/rationals.jl#L71

Added line #L71 was not covered by tests

# Rational division:
Expand Down
12 changes: 10 additions & 2 deletions test/integers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,16 @@
@testset verbose=true "Arithmetic" begin
a = Polymake.Integer(2)
@test -a == -2
# for T in [IntTypes; Polymake.Integer]
for T in IntTypes

@test sign(a) == 1
@test sign(-a) == -1
@test sign(a-a) == 0

@test abs(a) * sign(a) == a
@test abs(-a) * -sign(a) == -a

#for T in IntTypes
for T in [IntTypes; Polymake.Integer]
b = T(5)
# Equality
@test a == T(2)
Expand Down
13 changes: 12 additions & 1 deletion test/quadraticextension.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,18 @@
a /= b
@test a == Polymake.QuadraticExtension{Polymake.Rational}(735//40328, -85//40328, 5)
end


@testset verbose=true "sign abs" begin
a = Polymake.QuadraticExtension{Polymake.Rational}(5)
b = Polymake.QuadraticExtension{Polymake.Rational}(17, 1, 5)
@test sign(a) == 1
@test sign(b) == 1
@test sign(-b) == -1
@test sign(b-b) == 0

@test abs(a) * sign(a) == a
@test abs(-a) * -sign(a) == -a
end
end

@testset verbose=true "zero / one" begin
Expand Down
10 changes: 10 additions & 0 deletions test/rationals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@
end
end

@testset verbose=true "sign abs" begin
a = Polymake.Rational(2, 3)
@test sign(a) == 1
@test sign(-a) == -1
@test sign(a-a) == 0

@test abs(a) * sign(a) == a
@test abs(-a) * -sign(a) == -a
end

@testset verbose=true "zero / one" begin
ZERO = Polymake.Rational(0)
ONE = Polymake.Rational(1)
Expand Down

0 comments on commit b650ddc

Please sign in to comment.