Nemocas / AbstractAlgebra.jl

Generic abstract algebra functionality in pure Julia (no C dependencies)
https://nemocas.github.io/AbstractAlgebra.jl/dev/index.html
Other
171 stars 63 forks source link

Conformance tests use isapprox for inexact rings, that fails with padic #1001

Open fingolfin opened 3 years ago

fingolfin commented 3 years ago

In the ring conformance tests, we have this code:

function equality(a::T, b::T) where T <: AbstractAlgebra.NCRingElement
   if isexact_type(T)
      return a == b
   else
      return isapprox(a, b)
   end
end

However, this fails for padics as implemented in Nemo, because there is no isapprox method there. Looking at the relevant part of the AA Ring interface documentation, I am not sure if isapprox should be supported by padics; or if this function is only meant for rings using "floating point and ball arithmetic" -- IMHO the ring interface documentation should be clarified in that regard.

But that part of the documentation also says:

By default, inexact ring elements in AbstractAlgebra compare equal if they are the same to the minimum precision of the two elements.

Which makes me wonder why we need this helper function equality at all? @tthsqe12 ?

tthsqe12 commented 3 years ago

The original tests tested BigFloat and the == surely doesn't work there.

tthsqe12 commented 3 years ago

The plot thickens. I was thinking to define equality the tests simply as == and forget about julia's floats. But then I tried this. Clearly we have a lot of non-conforming rings currently in use.

julia> CC = AcbField(100)
Complex Field with 100 bits of precision and error bounds

julia> x = CC(1//3)
[0.333333333333333333333333333333 +/- 4.65e-31]

julia> y1=x*x*x*x
[0.012345679012345679012345679012 +/- 3.86e-31]

julia> y2=x^4
[0.012345679012345679012345679012 +/- 3.80e-31]

julia> y1 == y2
false

julia> isapprox(y1, y2)
ERROR: MethodError: no method matching isapprox(::acb, ::acb)

julia> overlaps(y1, y2)
true