Skip to content

Commit

Permalink
Add more exhaustive mutating op tests to ring conformance test (#1816)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lgoettgens
lgoettgens authored Oct 4, 2024
1 parent f1539ec commit b49039c
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 45 deletions.
2 changes: 1 addition & 1 deletion Project.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "AbstractAlgebra"
uuid = "c3fe647b-3220-5bb0-a1ea-a7954cac585d"
version = "0.43.2"
version = "0.43.3"

[deps]
InteractiveUtils = "b77e0a4c-d291-57a0-90e8-8db25a27a240"
Expand Down
179 changes: 135 additions & 44 deletions test/Rings-conformance-tests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,110 @@ function equality(a::T, b::T) where T <: AbstractAlgebra.NCRingElement
end
end

# The following functions should not expect that their input is a `NCRingElem` or similar.
# They should be usable in more general types, that don't even have a `parent/elem` correspondence
function test_mutating_op_like_zero(f::Function, f!::Function, A)
a = deepcopy(A)
a = f!(a)
@test equality(a, f(A))
end

function test_mutating_op_like_neg(f::Function, f!::Function, A)
# initialize storage var with different values to check that its value is not used
for z in [zero(A), deepcopy(A)]
a = deepcopy(A)
z = f!(z, a)
@test equality(z, f(A))
@test a == A
end

a = deepcopy(A)
a = f!(a)
@test equality(a, f(A))
end

function test_mutating_op_like_add(f::Function, f!::Function, A, B)
# initialize storage var with different values to check that its value is not used
for z in [zero(A), deepcopy(A), deepcopy(B)]
a = deepcopy(A)
b = deepcopy(B)
z = f!(z, a, b)
@test equality(z, f(A, B))
@test a == A
@test b == B
end

a = deepcopy(A)
b = deepcopy(B)
a = f!(a, a, b)
@test equality(a, f(A, B))
@test b == B

a = deepcopy(A)
b = deepcopy(B)
b = f!(b, a, b)
@test equality(b, f(A, B))
@test a == A

a = deepcopy(A)
b = deepcopy(B)
a = f!(a, b, b)
@test equality(a, f(B, B))
@test b == B

b = deepcopy(B)
b = f!(b, b, b)
@test equality(b, f(B, B))

a = deepcopy(A)
b = deepcopy(B)
a = f!(a, b)
@test equality(a, f(A, B))
@test b == B

b = deepcopy(B)
b = f!(b, b)
@test equality(b, f(B, B))
end

function test_mutating_op_like_addmul(f::Function, f!_::Function, Z, A, B)
f!(z, a, b, ::Nothing) = f!_(z, a, b)
f!(z, a, b, t) = f!_(z, a, b, t)

# initialize storage var with different values to check that its value is not used
# and `nothing` for the three-arg dispatch
for t in [nothing, zero(A), deepcopy(A)]
z = deepcopy(Z)
a = deepcopy(A)
b = deepcopy(B)
z = f!(z, a, b, t)
@test equality(z, f(Z, A, B))
@test a == A
@test b == B

a = deepcopy(A)
b = deepcopy(B)
a = f!(a, a, b, t)
@test equality(a, f(A, A, B))
@test b == B

a = deepcopy(A)
b = deepcopy(B)
b = f!(b, a, b, t)
@test equality(b, f(B, A, B))
@test a == A

a = deepcopy(A)
b = deepcopy(B)
a = f!(a, b, b, t)
@test equality(a, f(A, B, B))
@test b == B

b = deepcopy(B)
b = f!(b, b, b, t)
@test equality(b, f(B, B, B))
end
end

function test_NCRing_interface(R::AbstractAlgebra.NCRing; reps = 50)

Expand Down Expand Up @@ -82,7 +186,11 @@ function test_NCRing_interface(R::AbstractAlgebra.NCRing; reps = 50)
for i in 1:reps
a = test_elem(R)::T
@test hash(a) isa UInt
@test hash(a) == hash(deepcopy(a))
A = deepcopy(a)
@test !ismutable(a) || a !== A
@test equality(a, A)
@test hash(a) == hash(A)
@test parent(a) === parent(A)
@test sprint(show, "text/plain", a) isa String
end
@test sprint(show, "text/plain", R) isa String
Expand Down Expand Up @@ -171,50 +279,18 @@ function test_NCRing_interface(R::AbstractAlgebra.NCRing; reps = 50)
a = test_elem(R)::T
b = test_elem(R)::T
c = test_elem(R)::T
A = deepcopy(a)
B = deepcopy(b)
C = deepcopy(c)

test_mutating_op_like_zero(zero, zero!, a)
test_mutating_op_like_zero(one, one!, a)

x = deepcopy(a)
@test iszero(zero!(x))

ab = a*b
x = R()
@test mul!(x, a, b) == ab
x = deepcopy(b)
@test mul!(x, a, b) == ab
x = deepcopy(a)
@test mul!(x, x, b) == ab
x = deepcopy(b)
@test mul!(x, a, x) == ab

ab = a+b
x = R()
@test add!(x, a, b) == ab
x = deepcopy(b)
@test add!(x, a, b) == ab
x = deepcopy(a)
@test add!(x, x, b) == ab
x = deepcopy(b)
@test add!(x, a, x) == ab
x = deepcopy(a)
@test add!(x, b) == ab

# isapprox as BigFloat may fuse
# matrices don't implement addmul!
#t = R()
#x = deepcopy(a)
#@test equality(addmul!(x, b, c, t), a+b*c)
#x = deepcopy(a)
#@test equality(addmul!(x, x, c, t), a+a*c)
#x = deepcopy(a)
#@test equality(addmul!(x, b, x, t), a+b*a)
#x = deepcopy(a)
#@test equality(addmul!(x, x, x, t), a+a*a)
test_mutating_op_like_neg(-, neg!, a)

@test A == a
@test B == b
@test C == c
test_mutating_op_like_add(+, add!, a, b)
test_mutating_op_like_add(-, sub!, a, b)
test_mutating_op_like_add(*, mul!, a, b)

test_mutating_op_like_addmul((a, b, c) -> a + b*c, addmul!, a, b, c)
test_mutating_op_like_addmul((a, b, c) -> a - b*c, submul!, a, b, c)
end
end
end
Expand All @@ -234,12 +310,14 @@ function test_Ring_interface(R::AbstractAlgebra.Ring; reps = 50)
test_NCRing_interface(R)

@testset "Basic functionality for commutative rings only" begin
@test isone(AbstractAlgebra.inv(one(R)))
test_mutating_op_like_neg(AbstractAlgebra.inv, inv!, one(R))
test_mutating_op_like_neg(AbstractAlgebra.inv, inv!, -one(R))
for i in 1:reps
a = test_elem(R)::T
b = test_elem(R)::T
A = deepcopy(a)
B = deepcopy(b)
@test isone(AbstractAlgebra.inv(one(R)))
@test a*b == b*a
# documentation is not clear on divexact
if is_domain_type(T)
Expand All @@ -249,6 +327,7 @@ function test_Ring_interface(R::AbstractAlgebra.Ring; reps = 50)
if T isa RingElem
@test iszero(b) || equality((b*a) / b, a)
end
iszero(b) || test_mutating_op_like_add(divexact, divexact!, b*a, b)
else
try
t = divexact(b*a, b)
Expand Down Expand Up @@ -297,7 +376,14 @@ function test_Field_interface(R::AbstractAlgebra.Field; reps = 50)

for i in 1:reps
a = test_elem(R)::T
A = deepcopy(a)
@test is_unit(a) == !iszero(a)
if !is_zero(a)
@test is_one(a * inv(a))
@test is_one(inv(a) * a)
test_mutating_op_like_neg(inv, inv!, a)
end
@test A == a
end
end

Expand Down Expand Up @@ -385,6 +471,11 @@ function test_EuclideanRing_interface(R::AbstractAlgebra.Ring; reps = 20)
@test d == gcd(f, g)
@test d == s*f + t*g
@test gcdinv(f, g) == (d, s)

test_mutating_op_like_add(AbstractAlgebra.div, div!, f, m)
test_mutating_op_like_add(mod, mod!, f, m)
test_mutating_op_like_add(gcd, gcd!, f, m)
test_mutating_op_like_add(lcm, lcm!, f, m)
end

end
Expand Down

2 comments on commit b49039c

@lgoettgens
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/116717

Tip: Release Notes

Did you know you can add release notes too? Just add markdown formatted text underneath the comment after the text
"Release notes:" and it will be added to the registry PR, and if TagBot is installed it will also be added to the
release that TagBot creates. i.e.

@JuliaRegistrator register

Release notes:

## Breaking changes

- blah

To add them here just re-invoke and the PR will be updated.

Tagging

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.43.3 -m "<description of version>" b49039ce166e0d4ae9aa304d82c738d50792b1ae
git push origin v0.43.3

Please sign in to comment.