Add some ring / field conformance tests#1707
Conversation
aa0f0fa to
2a5446b
Compare
|
I probably misunderstood something, but |
| return sum( [r[i]*b[i] for i in 1:n]) | ||
| end | ||
|
|
||
| # TODO/FIXME: implement isapprox so we can get rid of the following HACK: |
There was a problem hiding this comment.
why? From the docs:
Approximation (floating point and ball arithmetic only)
isapprox(f::MyElem, g::MyElem; atol::Real=sqrt(eps()))so it is not part of the normal ring/field interface I'd say
There was a problem hiding this comment.
I never really questioned the use of isapprox in the conformance tests, I assumed that Dan had checked this was the right thing to use when he added them.
I'd be happy to use something else then, but what? We currently have this helper in the conformance tests:
# helper
function equality(a, b)
if is_exact_type(typeof(a)) && is_exact_type(typeof(b))
return a == b
else
return isapprox(a, b)
endWe use it to implement tests like these:
@test equality(a^4, a*a*a*a)
@test equality((a + b) + c, a + (b + c))
@test equality(a + b, b + a)
@test equality(a - c, a + (-c))
What would be a way to compare two QAdicFieldElem in a way that would satisfy such tests that deals with the problem that e.g. a^4 and a*a*a*a may produce slightly different results (e.g. perhaps a^4 is "more" accurate)
|
No I don't think you missed anything -- the general conformance tests should not require We'll fix the conformance test suite, and once that's there, I can adjust this PR (there are still a few valid changes in here, like a missing |
| @testset "RelFinField" begin | ||
|
|
||
| @testset "conformance" begin | ||
| F = Native.finite_field(3, 3, cached = false)[1] |
There was a problem hiding this comment.
@thofma I thought we'd rather get rid of the RelFinField?
There was a problem hiding this comment.
I only replaced the ones in the prime decomposition of relative number fields. They might be used somewhere else still.
|
Thanks for the clarification. I am also happy to help with missing methods. |
2a5446b to
364fe92
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1707 +/- ##
==========================================
+ Coverage 6.39% 6.46% +0.06%
==========================================
Files 361 361
Lines 114249 114640 +391
==========================================
+ Hits 7310 7414 +104
- Misses 106939 107226 +287
🚀 New features to boost your workflow:
|
0e8647a to
1dce07c
Compare
1dce07c to
c8b1da2
Compare
|
I forgot about this PR :-( But now it is updated. |
... and fix some issues this revealed. In particular "missing"base_ringmethodswhich the conformance tests kind of assume, but here in some cases "only"
base_fieldmethods were available.Contains #1706 because of overlap.