Sage: Ticket #10952: better numerical accuracy testing
https://trac.sagemath.org/ticket/10952
<p>
If a line contains <code>tol</code> or <code>tolerance</code>, numerical results are only
verified to the given tolerance. This may be prefixed by <code>abs[olute]</code> or <code>rel[ative]</code> to specify whether to measure absolute or relative error; defaults to relative error except when the expected value is exactly zero:
</p>
<pre class="wiki"> sage: RDF(pi) # abs tol 1e-5
3.14159
sage: [10^n for n in [0.0 .. 4]] # rel tol 2e-4
[0.9999, 10.001, 100.01, 999.9, 10001]
</pre><p>
This can be useful when the exact output is subject to rounding error and/or processor floating point arithmetic variation.
</p>
<hr />
<p>
Related:
</p>
<ul><li><code>.zero_at(epsilon)</code> methods, to fix noisy (and signed) zeroes; see for example <a class="closed ticket" href="https://trac.sagemath.org/ticket/11848" title="enhancement: zero_at() method for RDF/CDF vectors (closed: fixed)">#11848</a>.
</li></ul><hr />
<p>
Apply
</p>
<ol><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10952/10952-tol-bin.2.patch" title="Attachment '10952-tol-bin.2.patch' in Ticket #10952">10952-tol-bin.2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/10952/10952-tol-bin.2.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10952/trac_10952-ref.patch" title="Attachment 'trac_10952-ref.patch' in Ticket #10952">trac_10952-ref.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/10952/trac_10952-ref.patch" title="Download"></a>
</li></ol><p>
to the Sage <strong>scripts repository</strong>.
</p>
<p>
Apply
</p>
<ol><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10952/10952-tol-doc.2.patch" title="Attachment '10952-tol-doc.2.patch' in Ticket #10952">10952-tol-doc.2.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/10952/10952-tol-doc.2.patch" title="Download"></a>
</li><li><a class="attachment" href="https://trac.sagemath.org/attachment/ticket/10952/trac_10952-reviewer-docs-v3.patch" title="Attachment 'trac_10952-reviewer-docs-v3.patch' in Ticket #10952">trac_10952-reviewer-docs-v3.patch</a><a class="trac-rawlink" href="https://trac.sagemath.org/raw-attachment/ticket/10952/trac_10952-reviewer-docs-v3.patch" title="Download"></a>
</li></ol><p>
to the Sage library repository.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10952
Trac 1.1.6robertwbThu, 17 Mar 2011 08:11:34 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>noise.patch</em>
</li>
</ul>
TicketrobertwbThu, 17 Mar 2011 08:13:13 GMTstatus, description changed
https://trac.sagemath.org/ticket/10952#comment:1
https://trac.sagemath.org/ticket/10952#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=1">diff</a>)
</li>
</ul>
TicketjasonThu, 17 Mar 2011 13:45:04 GMT
https://trac.sagemath.org/ticket/10952#comment:2
https://trac.sagemath.org/ticket/10952#comment:2
<p>
Amazing speed getting this up! Shouldn't "((?:abs(?:solute)?)" be "((?:abs(?:olute)?)" ?
</p>
TicketjasonThu, 17 Mar 2011 13:51:02 GMT
https://trac.sagemath.org/ticket/10952#comment:3
https://trac.sagemath.org/ticket/10952#comment:3
<p>
Nitpicky: This comment is now incorrect:
</p>
<pre class="wiki"># following three: used only for parsing only_optional; list of comments
</pre><p>
Also, something should be added to the docs about this new option. Perhaps here: <a class="ext-link" href="http://sagemath.org/doc/developer/conventions.html#further-conventions-for-automated-testing-of-examples"><span class="icon"></span>http://sagemath.org/doc/developer/conventions.html#further-conventions-for-automated-testing-of-examples</a>
</p>
TicketrobertwbFri, 18 Mar 2011 06:30:31 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>10952-tol-bin.patch</em>
</li>
</ul>
<p>
apply only this patch to the bin repo
</p>
TicketrobertwbFri, 18 Mar 2011 06:30:47 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>10952-tol-doc.patch</em>
</li>
</ul>
TicketrobertwbFri, 18 Mar 2011 06:31:46 GMT
https://trac.sagemath.org/ticket/10952#comment:4
https://trac.sagemath.org/ticket/10952#comment:4
<p>
Thanks for the comments, new patches attached.
</p>
TicketkcrismanThu, 21 Apr 2011 01:48:09 GMTcc changed
https://trac.sagemath.org/ticket/10952#comment:5
https://trac.sagemath.org/ticket/10952#comment:5
<ul>
<li><strong>cc</strong>
<em>kcrisman</em> added
</li>
</ul>
TicketfbisseySun, 24 Apr 2011 03:14:50 GMT
https://trac.sagemath.org/ticket/10952#comment:6
https://trac.sagemath.org/ticket/10952#comment:6
<p>
Would this works with test of that kind as well:
</p>
<pre class="wiki">File "/usr/share/sage/devel/sage-main/sage/stats/hmm/chmm.pyx", line 579:
sage: m.viterbi([-2,-1,.1,0.3])
Expected:
([1, 1, 1, 0], -9.5660236533785135)
Got:
([1, 1, 1, 0], -9.566023653378513)
</pre><p>
or even
</p>
<pre class="wiki">File "/usr/share/sage/devel/sage-main/sage/categories/examples/semigroups.py", line 32:
sage: S.some_elements()
Expected:
[3, 42, 'a', 3.3999999999999999, 'raton laveur']
Got:
[3, 42, 'a', 3.4, 'raton laveur']
</pre><p>
(test failures courtesy of my work on python-2.7)
</p>
TicketrobertwbSun, 24 Apr 2011 04:58:20 GMT
https://trac.sagemath.org/ticket/10952#comment:7
https://trac.sagemath.org/ticket/10952#comment:7
<p>
Yes, it would (though you do have to explicitly annotate the test with a tolerance, and if Python 2.7 consistently gives, e.g., 3.4 then we should change the doctest rather than make it fuzzy.)
</p>
TicketmariahFri, 13 May 2011 14:19:23 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:8
https://trac.sagemath.org/ticket/10952#comment:8
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_info</em>
</li>
</ul>
<p>
In trying to review this ticket, I applied 10952-tol-bin.patch in the local/bin directory of a skynet/taurus (x86_64-Linux-nehalem) build of sage-4.7.rc2. I next did 'sage -b'. Yet I get
</p>
<pre class="wiki">sage: print "The answers are", 1.5, 2, 1e-12 # tol 1e-3
The answers are 1.50000000000000 2 1.00000000000000e-12
</pre><p>
Am I missing something?
</p>
TicketrobertwbSat, 14 May 2011 00:24:00 GMT
https://trac.sagemath.org/ticket/10952#comment:9
https://trac.sagemath.org/ticket/10952#comment:9
<p>
No, that is correct.
</p>
<p>
The example in the description is an example of something that would pass doctests (even though the true output doesn't match exactly, you're seeing what the actual output is).
</p>
TicketjasonSat, 14 May 2011 00:32:03 GMT
https://trac.sagemath.org/ticket/10952#comment:10
https://trac.sagemath.org/ticket/10952#comment:10
<p>
Another nitpicky: the doc patch misspells "arithmetic". Definitely not big enough to stop a positive review, but also not big enough to be worth my effort in making a new patch right now.
</p>
TicketrobertwbSat, 14 May 2011 00:36:26 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>10952-tol-doc.2.patch</em>
</li>
</ul>
TicketrobertwbSat, 14 May 2011 00:36:56 GMT
https://trac.sagemath.org/ticket/10952#comment:11
https://trac.sagemath.org/ticket/10952#comment:11
<p>
Argh. Fixed.
</p>
TicketjasonSat, 14 May 2011 00:39:48 GMT
https://trac.sagemath.org/ticket/10952#comment:12
https://trac.sagemath.org/ticket/10952#comment:12
<p>
Thanks! If we had a github pull system, or a system where I could edit the patch inline, I would have fixed it.
</p>
TicketkcrismanSat, 14 May 2011 11:22:36 GMTstatus changed; reviewer, author set
https://trac.sagemath.org/ticket/10952#comment:13
https://trac.sagemath.org/ticket/10952#comment:13
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jason Grout</em>
</li>
<li><strong>author</strong>
set to <em>Robert Bradshaw</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:12" title="Comment 12">jason</a>:
</p>
<blockquote class="citation">
<p>
Thanks! If we had a github pull system, or a system where I could edit the patch inline, I would have fixed it.
</p>
</blockquote>
<p>
I have to admit that <em>would</em> be convenient. But wouldn't that create a privileged class of commit people who don't need review? At least that seems to be the model for the projects I'm familiar with.
</p>
TicketjasonSat, 14 May 2011 15:02:00 GMT
https://trac.sagemath.org/ticket/10952#comment:14
https://trac.sagemath.org/ticket/10952#comment:14
<p>
That <em>person</em> would be the release manager. He would be the only one able (or supposed to) merge into the master branch.
</p>
<p>
On the other hand, given our problems with finding release managers who could do the entire release process, maybe sharing the burden to commit patches to master wouldn't be a bad idea.
</p>
TicketmariahMon, 16 May 2011 15:51:12 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:15
https://trac.sagemath.org/ticket/10952#comment:15
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
I applied 10952-tol-bin.patch in the local/bin directory of a skynet/taurus (x86_64-Linux-nehalem) build of sage-4.7.rc2. I next did 'sage -b'. I then modified a doctest to include the lines in
the ticket description
</p>
<pre class="wiki">sage: print "The answers are", 1.5, 2, 1e-12 # tol 1e-3
The answers are 1.499999 2.0001 0
</pre><p>
I then did 'sage -b' again. When I run the doctest, I get
</p>
<pre class="wiki">% ./sage -t -long -force_lib "devel/sage/sage/symbolic/units.py"
sage -t -long -force_lib "devel/sage/sage/symbolic/units.py"
**********************************************************************
File "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-nehalem-fc-review-10952/devel/sage/sage/symbolic/units.py", line 13:
sage: print "The answers are", 1.5, 2, 1e-12 # tol 1e-3
Exception raised:
Traceback (most recent call last):
File "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-nehalem-fc-review-10952/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-nehalem-fc-review-10952/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/home/mariah/sage/sage-4.7.rc2-x86_64-Linux-nehalem-fc-review-10952/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_0[6]>", line 1
res = print "The answers are", RealNumber('1.5'), Integer(2), RealNumber('1e-12') # tol 1e-3###line 13:
sage: print "The answers are", 1.5, 2, 1e-12 # tol 1e-3
^
SyntaxError: invalid syntax
**********************************************************************
</pre><p>
Note that if I put the lines
</p>
<pre class="wiki"> sage: RDF(pi) # abs tol 1e-5
3.14159
</pre><p>
in the doctest, then the doctest passes.
</p>
TicketrobertwbMon, 16 May 2011 19:38:54 GMTstatus, description changed
https://trac.sagemath.org/ticket/10952#comment:16
https://trac.sagemath.org/ticket/10952#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=16">diff</a>)
</li>
</ul>
<p>
True. It doesn't work with print (or other non-expression statements). I've created <a class="closed ticket" href="https://trac.sagemath.org/ticket/11336" title="defect: Update doctest tolerance to work with print statement. (closed: duplicate)">#11336</a> to generalize it.
</p>
<p>
I think that this is still plenty useful even with that limitation, and don't know when I'll have more time to work on it--we could either get this in and start using it now or hold off until has time to write a more complete implementation at some later date (which I'd like to get to someday, but that someday list is long...)
</p>
TicketmariahTue, 17 May 2011 13:25:19 GMTstatus, reviewer changed
https://trac.sagemath.org/ticket/10952#comment:17
https://trac.sagemath.org/ticket/10952#comment:17
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Jason Grout</em> to <em>Jason Grout, Mariah Lenox</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:16" title="Comment 16">robertwb</a>:
</p>
<blockquote class="citation">
<p>
True. It doesn't work with print (or other non-expression statements). I've created <a class="closed ticket" href="https://trac.sagemath.org/ticket/11336" title="defect: Update doctest tolerance to work with print statement. (closed: duplicate)">#11336</a> to generalize it.
</p>
<p>
I think that this is still plenty useful even with that limitation
</p>
</blockquote>
<p>
Agreed. I am just doing my "reviewer" duty.
</p>
<p>
Positive review.
</p>
TicketjdemeyerTue, 17 May 2011 15:33:50 GMTstatus, priority changed; milestone set
https://trac.sagemath.org/ticket/10952#comment:18
https://trac.sagemath.org/ticket/10952#comment:18
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_info</em>
</li>
<li><strong>priority</strong>
changed from <em>major</em> to <em>critical</em>
</li>
<li><strong>milestone</strong>
set to <em>sage-4.7.1</em>
</li>
</ul>
<p>
Which patches have to be applied?
</p>
TicketjdemeyerTue, 17 May 2011 15:48:13 GMT
https://trac.sagemath.org/ticket/10952#comment:19
https://trac.sagemath.org/ticket/10952#comment:19
<p>
I think the documentation should be expanded a bit to show less trivial examples, for example complex numbers or polynomials with non-exact coefficients.
</p>
TicketjdemeyerTue, 17 May 2011 15:48:49 GMT
https://trac.sagemath.org/ticket/10952#comment:20
https://trac.sagemath.org/ticket/10952#comment:20
<p>
Also matrices
</p>
TicketrbeezerMon, 22 Aug 2011 06:18:28 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>trac_10952-reviewer-docs.patch</em>
</li>
</ul>
TicketrbeezerMon, 22 Aug 2011 06:24:44 GMTstatus, description changed
https://trac.sagemath.org/ticket/10952#comment:21
https://trac.sagemath.org/ticket/10952#comment:21
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=21">diff</a>)
</li>
</ul>
<p>
I have added an "Apply" list for this ticket.
</p>
<p>
My patch fixes one typo in the documentation (a missing ]).
</p>
<p>
I have added some non-trivial tests.
</p>
<p>
Jeroen:
</p>
<ol><li> abs() for a matrix computes the determinant. Not useful for testing.
</li></ol><ol start="2"><li> Is there a class of polynomials with numerical coefficients? Does it have abs() defined on it?
</li></ol><p>
I am working on this as a high-priority ticket for Bug Days 32.
</p>
<p>
Rob
</p>
TicketwasWed, 24 Aug 2011 05:39:48 GMTdescription changed
https://trac.sagemath.org/ticket/10952#comment:22
https://trac.sagemath.org/ticket/10952#comment:22
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=22">diff</a>)
</li>
</ul>
<p>
This ticket is pretty amazing.
</p>
<p>
Rob, this doctest you added seems very wrong:
</p>
<pre class="wiki">
A relative tolerance on a root of a polynomial.
::
sage: p = (y - 10^16)*(y-10^(-13))*(y-2); p
y^3 + (-1e+16)*y^2 + (2e+16)*y - 2000.0
sage: p.roots()
sage: p.roots(multiplicities=False)[2] # relative tol 1e-10
10^16
</pre><p>
In particular,
</p>
<ul><li>it seems that y isn't defined
</li><li>the output of <code>p.roots()</code> is missing
</li><li>I get an output of <code>1e16</code> instead of <code>10^16</code>.
</li></ul>
TicketrbeezerWed, 24 Aug 2011 05:49:55 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:23
https://trac.sagemath.org/ticket/10952#comment:23
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:22" title="Comment 22">was</a>:
</p>
<blockquote class="citation">
<p>
Rob, this doctest you added seems very wrong:
</p>
</blockquote>
<p>
Yes, I think this was the patch missing a refresh. I'll clean it in the AM.
</p>
TicketrbeezerWed, 24 Aug 2011 07:01:30 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>trac_10952-reviewer-docs-v2.patch</em>
</li>
</ul>
TicketrbeezerWed, 24 Aug 2011 07:08:36 GMTstatus, description changed
https://trac.sagemath.org/ticket/10952#comment:24
https://trac.sagemath.org/ticket/10952#comment:24
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=24">diff</a>)
</li>
</ul>
<p>
Reviewer documentation v2 patch cleans up the mess.
</p>
<p>
I used <code>10^16</code> intentionally. The tolerance feature does <em>arithmetic</em> with the
output, so does not have to identically match as text. Maybe that deserves a comment?
</p>
<p>
Rob
</p>
TicketrbeezerWed, 24 Aug 2011 16:11:08 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>trac_10952-reviewer-docs-v3.patch</em>
</li>
</ul>
TicketrbeezerWed, 24 Aug 2011 16:15:11 GMTdescription changed
https://trac.sagemath.org/ticket/10952#comment:25
https://trac.sagemath.org/ticket/10952#comment:25
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=25">diff</a>)
</li>
</ul>
<p>
Reviewer documentation v3 patch adds a comment about intended output being of any form, fixes some missing quotes and expands a bit more on how this works. Passes tests, so should be ready for review.
</p>
TicketwasWed, 24 Aug 2011 16:38:07 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:26
https://trac.sagemath.org/ticket/10952#comment:26
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Nice. Game changer!!!! Sweet. It's about time. Great work! W00t. FTW. Frickin' awesome.
</p>
TicketkcrismanWed, 24 Aug 2011 17:39:24 GMTreviewer, author changed
https://trac.sagemath.org/ticket/10952#comment:27
https://trac.sagemath.org/ticket/10952#comment:27
<ul>
<li><strong>reviewer</strong>
changed from <em>Jason Grout, Mariah Lenox</em> to <em>Jason Grout, Mariah Lenox, William Stein</em>
</li>
<li><strong>author</strong>
changed from <em>Robert Bradshaw</em> to <em>Robert Bradshaw, Rob Beezer</em>
</li>
</ul>
TicketwasWed, 24 Aug 2011 23:44:43 GMTkeywords set
https://trac.sagemath.org/ticket/10952#comment:28
https://trac.sagemath.org/ticket/10952#comment:28
<ul>
<li><strong>keywords</strong>
<em>sd32</em> added
</li>
</ul>
TicketleifThu, 08 Sep 2011 23:48:20 GMTdescription changed
https://trac.sagemath.org/ticket/10952#comment:29
https://trac.sagemath.org/ticket/10952#comment:29
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=29">diff</a>)
</li>
</ul>
<p>
Shouldn't the code only be included if the doctests actually contain <code># tol ...</code>?
</p>
<p>
<code>( +[0-9.e+-]+)?</code> hardly matches what we want btw.
</p>
<p>
<br />
</p>
<p>
I've put this onto the 1337 ticket (<a class="closed ticket" href="https://trac.sagemath.org/ticket/11337" title="task: Metaticket: improve doctesting framework (closed: invalid)">#11337</a>).
</p>
TicketjhpalmieriFri, 09 Sep 2011 03:08:59 GMTstatus, reviewer changed
https://trac.sagemath.org/ticket/10952#comment:30
https://trac.sagemath.org/ticket/10952#comment:30
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Jason Grout, Mariah Lenox, William Stein</em> to <em>Jason Grout, Mariah Lenox, William Stein, John Palmieri</em>
</li>
</ul>
<p>
This doesn't work. Re Leif's comment, I think the regexp should something like <code>' ((\.[0-9]+|[0-9]+(\.[0-9]*)?)e[+-]?[0-9]+)'</code>, which adds another group to the match, so we need to make this change:
</p>
<div class="wiki-code"><div xmlns="http://www.w3.org/1999/xhtml" class="diff">
<ul class="entries">
<li class="entry">
<h2>
<a>sage-doctest</a>
</h2>
<pre>diff --git a/sage-doctest b/sage-doctest</pre>
<table class="trac-diff inline" summary="Differences" cellspacing="0">
<colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
<thead>
<tr>
<th title="File a/sage-doctest">
a
</th>
<th title="File b/sage-doctest">
b
</th>
<td><em> if __name__ == '__main__':</em> </td>
</tr>
</thead>
<tbody class="unmod">
<tr>
<th>217</th><th>217</th><td class="l"><span>""" % dict</span></td>
</tr><tr>
<th>218</th><th>218</th><td class="l"><span></span></td>
</tr><tr>
<th>219</th><th>219</th><td class="l"><span>NONE=0; LONG_TIME=1; RANDOM=2; OPTIONAL=3; NOT_IMPLEMENTED=4; NOT_TESTED=5; TOLERANCE=6</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>220</th><th> </th><td class="l"><span>tolerance_pattern = re.compile(r'\b((?:abs(?:olute)?)|(?:rel(?:ative)?))? *?tol(?:erance)?\b<del>( +[0-9.e+-</del>]+)?')</span></td>
</tr>
<tr class="last">
<th> </th><th>220</th><td class="r"><span>tolerance_pattern = re.compile(r'\b((?:abs(?:olute)?)|(?:rel(?:ative)?))? *?tol(?:erance)?\b<ins> +((\.[0-9]+|[0-9]+(\.[0-9]*)?)e[+-]?[0-9</ins>]+)?')</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>221</th><th>221</th><td class="l"><span></span></td>
</tr><tr>
<th>222</th><th>222</th><td class="l"><span>def comment_modifier(s):</span></td>
</tr><tr>
<th>223</th><th>223</th><td class="l"><span> sind = s.find('#')</span></td>
</tr>
</tbody>
<tbody class="skipped">
<tr>
<th><a href="#L239">…</a></th>
<th><a href="#L239">…</a></th>
<td><em> def comment_modifier(s):</em> </td>
</tr>
</tbody>
<tbody class="unmod">
<tr>
<th>239</th><th>239</th><td class="l"><span> m = tolerance_pattern.search(L)</span></td>
</tr><tr>
<th>240</th><th>240</th><td class="l"><span> if m:</span></td>
</tr><tr>
<th>241</th><th>241</th><td class="l"><span> v.append(TOLERANCE)</span></td>
</tr>
</tbody><tbody class="mod">
<tr class="first">
<th>242</th><th> </th><td class="l"><span> rel_or_abs, epsilon<del></del> = m.groups()</span></td>
</tr>
<tr class="last">
<th> </th><th>242</th><td class="r"><span> rel_or_abs, epsilon<ins>, _, _</ins> = m.groups()</span></td>
</tr>
</tbody><tbody class="unmod">
<tr>
<th>243</th><th>243</th><td class="l"><span> if rel_or_abs is not None:</span></td>
</tr><tr>
<th>244</th><th>244</th><td class="l"><span> rel_or_abs = rel_or_abs[:3]</span></td>
</tr><tr>
<th>245</th><th>245</th><td class="l"><span> if epsilon is None:</span></td>
</tr>
</tbody>
</table>
</li>
</ul>
</div></div><p>
I think this should match any of "1.2e12", "1e-12", ".1e12", without matching "ee12e+-122", like the previous regexp.
</p>
<p>
In addition to this, the whole thing just flat doesn't work. I created a Python file with some tolerance tests in it, and they all fail, and the failures print badly: they include tracebacks, for example. I'm not sure why the rst file patched in the two "doc" patches isn't being doctested correctly, but as far as I can tell, no doctests are being run in that file -- just add an obviously wrong doctest, and tests still pass. Here is a test file:
</p>
<div class="wiki-code"><div class="code"><pre><span class="sd">"""
Here are some examples::
sage: 3+3
6
sage: RDF(pi) # abs tol 1e-4
3.14159
Here is another example::
sage: [10^n for n in [0.0 .. 4]] # rel tol 2e-4
[0.9999, 10.001, 100.01, 999.9, 10001]
And another::
sage: RDF(pi) # abs tol 1e-4
3.14159
And that's the end.
"""</span>
</pre></div></div><p>
Save this as "new.py" and run "sage -t new.py" to see some bad behavior.
</p>
TicketleifFri, 09 Sep 2011 04:00:06 GMT
https://trac.sagemath.org/ticket/10952#comment:31
https://trac.sagemath.org/ticket/10952#comment:31
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:30" title="Comment 30">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I think the regexp should something like
</p>
</blockquote>
<pre class="wiki">' ((\.[0-9]+|[0-9]+(\.[0-9]*)?)e[+-]?[0-9]+)'`
</pre><p>
But that doesn't match <code>1ee7</code> :(
</p>
<p>
You can of course match funny things first, i.e. use more general patterns, but you then have to check the matched expression further before passing it to <code>float()</code>.
</p>
<p>
The whole idea isn't that bad, but carries the danger that people simply increase the tolerance (too much) just to make doctests pass <em>somehow</em>, without caring where the variations originate from.
</p>
TicketjhpalmieriFri, 09 Sep 2011 05:01:22 GMT
https://trac.sagemath.org/ticket/10952#comment:32
https://trac.sagemath.org/ticket/10952#comment:32
<p>
But <code>float(1ee7)</code> raises an error. Why would we want to match that?
</p>
<p>
Re the issue of raising tolerance, I hope that referees will keep an eye on that sort of thing. Dave Kirkby, for example, is a strong advocate of not arbitrarily raising tolerance.
</p>
TicketrobertwbFri, 09 Sep 2011 05:20:21 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>10952-tol-bin.2.patch</em>
</li>
</ul>
TicketrobertwbFri, 09 Sep 2011 05:26:57 GMTdescription changed
https://trac.sagemath.org/ticket/10952#comment:33
https://trac.sagemath.org/ticket/10952#comment:33
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=33">diff</a>)
</li>
</ul>
<p>
I've fixed the issue with blanklines terminating example blocks. As for the regular expression for floats, I don't see any need to make it more complicated--I'd rather match miss-typed numbers and raise an error than silently ignore them. If you feel strongly, this could be changed.
</p>
<p>
Referees should keep an eye on precision. This is back to my philosophy that a computer should run doctests and a human read the code (though we could add a patchbot plugin to flag drops in precision). It's not a new issue--we've been using "..." for quite a while now which is less flexible and more dangerous (as it can match more than just a single number).
</p>
TicketleifFri, 09 Sep 2011 05:36:00 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:34
https://trac.sagemath.org/ticket/10952#comment:34
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketleifFri, 09 Sep 2011 05:48:36 GMT
https://trac.sagemath.org/ticket/10952#comment:35
https://trac.sagemath.org/ticket/10952#comment:35
<p>
How about emitting the extra code only if <code># tol ...</code> is used at all in the file?
</p>
<p>
That's a simple <code>"""... %s ...""" % (tolerance_code if uses_tol else "")</code>.
</p>
TicketrobertwbFri, 09 Sep 2011 06:15:53 GMT
https://trac.sagemath.org/ticket/10952#comment:36
https://trac.sagemath.org/ticket/10952#comment:36
<p>
You're talking about the <code>check_with_tolerance</code> function and helpers. One could add a variable to keep track of this and conditionally emit it; not sure if the added complexity is worth the tiny savings.
</p>
TicketleifFri, 09 Sep 2011 06:28:28 GMT
https://trac.sagemath.org/ticket/10952#comment:37
https://trac.sagemath.org/ticket/10952#comment:37
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:36" title="Comment 36">robertwb</a>:
</p>
<blockquote class="citation">
<p>
You're talking about the <code>check_with_tolerance</code> function and helpers. One could add a variable to keep track of this and conditionally emit it; not sure if the added complexity is worth the tiny savings.
</p>
</blockquote>
<p>
Added complexity? One boolean variable, initialized to false and set to <code>True</code> in only a few places?
</p>
<p>
This not only reduces the file size and run (import!) time but also avoids more potential name clashes.
</p>
<p>
Why should one add useless code to each and every file to doctest not using <code># tol</code>?
</p>
TicketrobertwbFri, 09 Sep 2011 08:49:12 GMT
https://trac.sagemath.org/ticket/10952#comment:38
https://trac.sagemath.org/ticket/10952#comment:38
<p>
Either we make this variable a global (ugly) or return it on from call to doc_preparse along with the parsed docstring (also ugly). All to save a fraction of a block (~1KB) of an ephemeral file and < 1ms on an already fairly expensive operation. If we're looking to cut fat, it'd be better to avoid the quadratic-time creation of the doctest file string.
</p>
<p>
The name clash is a red herring--if we avoid it only because # tol is not used, that's a potentially latent bug in my mind. The function could be renamed if need be.
</p>
TicketjhpalmieriFri, 09 Sep 2011 22:09:48 GMTdescription changed
https://trac.sagemath.org/ticket/10952#comment:39
https://trac.sagemath.org/ticket/10952#comment:39
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=39">diff</a>)
</li>
</ul>
<p>
I don't see anything wrong with more complicated regular expressions, but I like regular expressions. So leave it as is if you want.
</p>
<p>
Meanwhile, the output for failures using tolerances isn't nice, compared to other sorts of failures:
</p>
<pre class="wiki">sage -t "builds/sage-4.7.2.alpha2/devel/sage-new/sage/homology/new.py"
**********************************************************************
File "/Applications/sage_builds/sage-4.7.2.alpha2/devel/sage-new/sage/homology/new.py", line 4:
sage: 3+3
Expected:
8
Got:
6
**********************************************************************
File "/Applications/sage_builds/sage-4.7.2.alpha2/devel/sage-new/sage/homology/new.py", iled example:
check_with_tolerance('''
3.14159
''', res, 9.9999999999999998e-13, 'abs')
Exception raised:
Traceback (most recent call last):
File "/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_0[11]>", line 3, in <module>
''', res, 9.9999999999999998e-13, 'abs')
File "/Users/palmieri/.sage//tmp/new.py", line 35, in check_with_tolerance
assert abs(expected_value - actual_value) < epsilon, "Out of tolerance %s vs %s" % (expected_value, actual_value)
AssertionError: Out of tolerance 3.14159 vs 3.14159265359
**********************************************************************
1 items had failures:
2 of 13 in __main__.example_0
***Test Failed*** 2 failures.
For whitespace errors, see the file /Users/palmieri/.sage//tmp/.doctest_new.py
[3.9 s]
</pre><p>
Notice that the line number is missing and the traceback is present. I'm attaching a referee patch which tries to fix this. Please take a look. For me, this gives this sort of output (with a slightly different file):
</p>
<pre class="wiki">sage -t "builds/sage-4.7.2.alpha2/devel/sage-new/new.py"
**********************************************************************
File "/Applications/sage_builds/sage-4.7.2.alpha2/devel/sage-new/new.py", line 4:
sage: 3+3
Expected:
8
Got:
6
**********************************************************************
File "/Applications/sage_builds/sage-4.7.2.alpha2/devel/sage-new/new.py", line 6:
sage: RDF(pi) # abs tol 1e-6
Out of tolerance 3.14159 vs 3.14159265359
**********************************************************************
File "/Applications/sage_builds/sage-4.7.2.alpha2/devel/sage-new/new.py", line 16:
sage: RDF(pi) # abs tol 1e-8
Out of tolerance 3.14159 vs 3.14159265359
**********************************************************************
1 items had failures:
3 of 16 in __main__.example_0
***Test Failed*** 3 failures.
For whitespace errors, see the file /Users/palmieri/.sage//tmp/.doctest_new.py
[4.2 s]
</pre><p>
Since the whole ticket had a positive review earlier, I think if you're happy with my changes, you can switch it back to that status.
</p>
<p>
Finally, the issue with doctests in <code>conventions.rst</code> is, I think, that the various triple quotes <code>"""</code> confuse the parsing routine (the function <code>pythonify_rst</code> in particular). This belongs on another ticket. That file contains enough incomplete code stubs that I wonder if it shouldn't be doctested at all...
</p>
TicketjhpalmieriFri, 09 Sep 2011 22:10:12 GMTattachment set
https://trac.sagemath.org/ticket/10952
https://trac.sagemath.org/ticket/10952
<ul>
<li><strong>attachment</strong>
set to <em>trac_10952-ref.patch</em>
</li>
</ul>
<p>
scripts repo; apply on top of other scripts patch
</p>
TicketleifFri, 09 Sep 2011 22:31:48 GMT
https://trac.sagemath.org/ticket/10952#comment:40
https://trac.sagemath.org/ticket/10952#comment:40
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:39" title="Comment 39">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I don't see anything wrong with more complicated regular expressions, but I like regular expressions. So leave it as is if you want.
</p>
</blockquote>
<p>
Yes, only
</p>
<pre class="wiki">sage: foo # tol 1ee7
anything
</pre><p>
should always pass. ;-)
</p>
<p>
<br />
</p>
<blockquote class="citation">
<p>
Meanwhile, the output for failures using tolerances isn't nice, compared to other sorts of failures [...] <br />
Notice that the line number is missing and the traceback is present. I'm attaching a referee patch which tries to fix this. Please take a look.
</p>
</blockquote>
<p>
Haven't tested your patch (nor looked at it), but the output you gave looks much better.
</p>
<p>
<br />
</p>
<blockquote class="citation">
<p>
Since the whole ticket had a positive review earlier, I think if you're happy with my changes, you can switch it back to that status.
</p>
</blockquote>
<p>
I won't object, though I in general don't like the idea of adding code (regardless how small the overhead or impact might be) unconditionally, i.e., if there's no need to do so.
</p>
<p>
We have similar situations all around, where everybody adds "just a little", IMHO.
</p>
TicketrobertwbThu, 22 Sep 2011 21:36:08 GMTstatus changed
https://trac.sagemath.org/ticket/10952#comment:41
https://trac.sagemath.org/ticket/10952#comment:41
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:40" title="Comment 40">leif</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10952#comment:39" title="Comment 39">jhpalmieri</a>:
</p>
<blockquote class="citation">
<p>
I don't see anything wrong with more complicated regular expressions, but I like regular expressions. So leave it as is if you want.
</p>
</blockquote>
<p>
Yes, only
</p>
<pre class="wiki">sage: foo # tol 1ee7
anything
</pre><p>
should always pass. ;-)
</p>
</blockquote>
<p>
Currently, it raises an error indicating that the *doctest* itself is bad.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
Meanwhile, the output for failures using tolerances isn't nice, compared to other sorts of failures [...] <br />
Notice that the line number is missing and the traceback is present. I'm attaching a referee patch which tries to fix this. Please take a look.
</p>
</blockquote>
<p>
Haven't tested your patch (nor looked at it), but the output you gave looks much better.
</p>
<p>
<br />
</p>
<blockquote class="citation">
<p>
Since the whole ticket had a positive review earlier, I think if you're happy with my changes, you can switch it back to that status.
</p>
</blockquote>
<p>
I won't object, though I in general don't like the idea of adding code (regardless how small the overhead or impact might be) unconditionally, i.e., if there's no need to do so.
</p>
<p>
We have similar situations all around, where everybody adds "just a little", IMHO.
</p>
</blockquote>
<p>
Yes, it's a tradeoff between adding "just a little" python parsing overhead to the testing infrastructure or "just a little" human parsing overhead to the testing infrastructure. I'd rather put the burden on machines than developers in this case.
</p>
<p>
The reviewer patches look good, certainly an improvement, so I'm setting this back to positive review unless there's something else that needs to be taken care of.
</p>
TicketleifMon, 26 Sep 2011 02:00:02 GMTkeywords, description changed
https://trac.sagemath.org/ticket/10952#comment:42
https://trac.sagemath.org/ticket/10952#comment:42
<ul>
<li><strong>keywords</strong>
<em>noise</em> <em>noisy</em> <em>doctest</em> <em>failure</em> <em>error</em> <em>tolerance</em> added
</li>
<li><strong>description</strong>
modified (<a href="/ticket/10952?action=diff&version=42">diff</a>)
</li>
</ul>
TicketleifTue, 27 Sep 2011 17:41:07 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/10952#comment:43
https://trac.sagemath.org/ticket/10952#comment:43
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-4.7.2.alpha3</em>
</li>
</ul>
TicketkcrismanFri, 10 Feb 2012 20:06:36 GMT
https://trac.sagemath.org/ticket/10952#comment:44
https://trac.sagemath.org/ticket/10952#comment:44
<p>
I'd like to draw the attention of folks here to <a class="closed ticket" href="https://trac.sagemath.org/ticket/12493#comment:7" title="Comment 7 for Ticket #12493">comment:7:ticket:12493</a>. Apparently one can't do <code>only-optional</code> tests along with <code>tol</code> tests at the same time. My guess is that not too many people use <code>only-optional</code> and the <code>tol</code> stuff is pretty new.
</p>
<p>
In fact, I only found one occurrence in 5.0.beta3. Can that be right? This has been in Sage for months!
</p>
<pre class="wiki">sage: search_src(" tol ","#")
symbolic/integration/integral.py:587: sage: error.numerical_approx() # abs tol 10e-10
</pre><p>
Anyway, even if my analysis is wrong (let's hope it's easier than that), I figure the people here can give a quick diagnosis of <a class="closed ticket" href="https://trac.sagemath.org/ticket/12493" title="defect: tol and optional in doctests don't play well together (closed: fixed)">#12493</a>.
</p>
TicketkcrismanFri, 10 Feb 2012 20:07:22 GMT
https://trac.sagemath.org/ticket/10952#comment:45
https://trac.sagemath.org/ticket/10952#comment:45
<p>
Actually, <a class="closed ticket" href="https://trac.sagemath.org/ticket/12493#comment:8" title="Comment 8 for Ticket #12493">comment:8:ticket:12493</a> is even better! Optional and tol don't play well together.
</p>
Ticket