Opened 6 years ago
Closed 3 years ago
#21121 closed defect (fixed)
Precision in doctest tolerance: x > x
Reported by:  leif  Owned by:  

Priority:  minor  Milestone:  sage8.4 
Component:  doctest framework  Keywords:  doctest tolerance exceeded 
Cc:  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Marc Mezzarobba 
Report Upstream:  N/A  Work issues:  
Branch:  d7f0862 (Commits, GitHub, GitLab)  Commit:  d7f08627df8903903f2b42ed461e4ba0b477b216 
Dependencies:  Stopgaps: 
Description (last modified by )
Failed example: A.exp() # tol 1e14 Expected: [19.614602953804912 + 12.517743846762578*I 3.7949636449582176 + 28.88379930658099*I] [ 32.383580980922254 + 21.88423595789845*I 2.269633004093535 + 44.901324827684824*I] Got: [19.614602953804923 + 12.517743846762574*I 3.7949636449582007 + 28.883799306580993*I] [32.383580980922275 + 21.884235957898447*I 2.2696330040935115 + 44.901324827684846*I] Tolerance exceeded in 1 of 8: 2.269633004093535 vs 2.2696330040935115, tolerance 1e14 > 1e14
Sage saying that 10^{14} > 10^{14} appears a bit stupid.
The attached branch fixes this by displaying y > x
where:
x
is the required tolerance (given in the doctest) with 15 digits, rounded to nearest.
y
is the observed tolerance (computed by running the test) with 1 digit, rounded up. This means that using# tol y
is guaranteed to make the test pass.
Assuming that the tolerance in the doctest is given to at most 15 digits of precision (almost certainly the case), x
should be exactly what was in the doctest. In that case, y
cannot be equal to x
.
We also refactor the implementation for printing tolerance errors somewhat.
Finally, we use 1044 bits of precision to compute tolerances. This may seem overkill, but it is needed if we want to deal with tolerances on numbers computed with 1024 bits of precision.
Change History (28)
comment:1 Changed 6 years ago by
 Dependencies set to #21124
comment:2 Changed 6 years ago by
 Description modified (diff)
comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
(As is, failstr()
truncates the mantissa to just one digit, i.e., the digit before the decimal point.)
comment:5 Changed 6 years ago by
I.e., we should at least make sure "%.0e" % RIFtol(actual).center()
doesn't give the same as "%.0e" % RIFtol(desired).center()
. (Not sure whether using .center()
here is at all the right thing, or whether we should at all use RIF
for the tolerance values themselves; see also below.)
If it does, we may increase the number of (mantissa) digits in the former. Or in both, until they differ.
And that's orthogonal to improving hints regarding the actual deviation, i.e., if one wants to adjust the expected figures rather than (just) the tolerance.
If the intervals of want and got do not overlap (i.e., when the tolerance is exceeded), we could also give .upper()
and .lower()
(or vice versa, depending on whether want is smaller or larger than got).
comment:6 Changed 6 years ago by
 Description modified (diff)
comment:7 Changed 6 years ago by
With

src/sage/doctest/parsing.py
diff git a/src/sage/doctest/parsing.py b/src/sage/doctest/parsing.py index b297bb1..def1e31 100644
a b class SageOutputChecker(doctest.OutputChecker): 963 963 want_intervals = [self.add_tolerance(v, want) for v in want_values] 964 964 got_values = [RIFtol(g) for g in got_str] 965 965 if len(want_values) == len(got_values): 966 966 967 def failstr(astr, bstr, actual, desired): 967 return " %s vs %s, tolerance %.0e > %.0e"%(astr, bstr, RIFtol(actual).center(), RIFtol(desired). 968 # Do not print 'x > x' (#21121): 969 dp = 0 # start with zero decimal places in output of tolerance 970 while True: # XXX kind of dangerous... 971 allowed_tol = "%.*e" % (dp, RIFtol(desired).center()) 972 effective_tol = "%.*e" % (dp, RIFtol(actual).center()) 973 if allowed_tol != effective_tol: 974 # we achieved meaningful output 975 break 976 dp += 1 # increase number of decimal places to be shown 977 978 return " %s vs %s, tolerance %s > %s"%(astr, bstr, effective_tol, allowed_tol) 968 979 969 980 fails = [] 970 981 for a, ainterval, b, astr, bstr in zip(want_values, want_intervals, got_values, want_str, got_str):
I would for example get the following for the failing test from the ticket's description / #21119:
sage t src/sage/matrix/matrix_double_dense.pyx ********************************************************************** File "src/sage/matrix/matrix_double_dense.pyx", line 3761, in sage.matrix.matrix_double_dense.Matrix_double_dense.exp Failed example: A.exp() # tol 1e14 Expected: [19.614602953804912 + 12.517743846762578*I 3.7949636449582176 + 28.88379930658099*I] [ 32.383580980922254 + 21.88423595789845*I 2.269633004093535 + 44.901324827684824*I] Got: [19.614602953804923 + 12.517743846762574*I 3.7949636449582007 + 28.883799306580993*I] [32.383580980922275 + 21.884235957898447*I 2.2696330040935115 + 44.901324827684846*I] Tolerance exceeded in 1 of 8: 2.269633004093535 vs 2.2696330040935115, tolerance 1.04e14 > 1.00e14 **********************************************************************
AFAICT, we unfortunately there do not know whether # tol 1e14
or e.g. # tol 1.0e14
was specified in the failing doctest.
comment:8 Changed 6 years ago by
To do:
Replace True
by dp <= some_meaningful_number
and break
by return ...
(returning what's now returned below, after the loop).
Then if we exit the while
loop (giving up on too many decimal places), fall back to returning some other (meaningful) output, of course again other than ... x > x
.
comment:9 Changed 6 years ago by
... and the mentioned doctests for the doctest framework itself of course need to get adjusted to the new output.
comment:10 Changed 6 years ago by
You just need to print the numbers with directed roundings (up/down). But that needs #21124 to properly print the numbers.
comment:11 Changed 4 years ago by
 Branch set to u/jdemeyer/precision_in_doctest_tolerance__x___x
comment:12 Changed 4 years ago by
 Commit set to a9909ab60eb35282534e545ee1e7ec633601c779
 Dependencies changed from #21124 to #26154
New commits:
a9909ab  Improve output formatting for tolerances

comment:13 Changed 4 years ago by
 Commit changed from a9909ab60eb35282534e545ee1e7ec633601c779 to e1ba5b73f39a47446d01581550ccf2a4259f14ca
comment:14 Changed 4 years ago by
 Commit changed from e1ba5b73f39a47446d01581550ccf2a4259f14ca to a636d1ee3c389f7270652db664e4b5ecfc388fd2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a636d1e  Improve output formatting for tolerances

comment:15 Changed 4 years ago by
 Description modified (diff)
 Keywords tolerance added; error removed
 Milestone changed from sage7.4 to sage8.4
 Status changed from new to needs_review
comment:16 Changed 4 years ago by
 Description modified (diff)
comment:17 Changed 4 years ago by
 Commit changed from a636d1ee3c389f7270652db664e4b5ecfc388fd2 to cb741123f8dec1849052c88b47dbc50caf3f6ab1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cb74112  Improve output formatting for tolerances

comment:18 Changed 4 years ago by
 Description modified (diff)
comment:19 Changed 4 years ago by
 Description modified (diff)
comment:20 Changed 4 years ago by
 Commit changed from cb741123f8dec1849052c88b47dbc50caf3f6ab1 to dd60a2e10199532a02bf591a41644df64d9ebef6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
dd60a2e  Improve output formatting for tolerances

comment:21 Changed 4 years ago by
 Commit changed from dd60a2e10199532a02bf591a41644df64d9ebef6 to 5535d2de60d94021d0332369e9cbe9ddc9e32cfd
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5535d2d  Improve output formatting for tolerances

comment:22 Changed 3 years ago by
 Commit changed from 5535d2de60d94021d0332369e9cbe9ddc9e32cfd to b76623dc03e4d81ff303eac219ad5c907a4c2360
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b76623d  Improve output formatting for tolerances

comment:23 Changed 3 years ago by
 Dependencies #26154 deleted
comment:24 Changed 3 years ago by
 Commit changed from b76623dc03e4d81ff303eac219ad5c907a4c2360 to e62891b0edc5754465df192ebb174938a7706e9e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
e62891b  Improve output formatting for tolerances

comment:25 Changed 3 years ago by
 Reviewers set to Marc Mezzarobba
 Status changed from needs_review to positive_review
LGTM (except that “it is carries” in the comment should be “it carries”).
comment:26 Changed 3 years ago by
 Commit changed from e62891b0edc5754465df192ebb174938a7706e9e to d7f08627df8903903f2b42ed461e4ba0b477b216
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:
d7f0862  Improve output formatting for tolerances

comment:28 Changed 3 years ago by
 Branch changed from u/jdemeyer/precision_in_doctest_tolerance__x___x to d7f08627df8903903f2b42ed461e4ba0b477b216
 Resolution set to fixed
 Status changed from positive_review to closed
The culprit (at least regarding the weird ouput) is here, namely in the local function
failstr()
:(A method of class
SageOutputChecker
insrc/sage/doctest/parsing.py
.)One could also change the code below its definition rather than (just)
failstr()
.