#16889 closed defect (fixed)
Doctest tolerance: allow spaces and use intervals
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-6.4 |
Component: | doctest framework | Keywords: | |
Cc: | zimmerma | Merged in: | sage-6.4.beta3 |
Authors: | Jeroen Demeyer | Reviewers: | Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | 1f858c7 (Commits, GitHub, GitLab) | Commit: | 1f858c7511d147e2f93dd0fd4e160fed8ad4f234 |
Dependencies: | Stopgaps: |
Description (last modified by )
The doctesting framework should ignore whitespace before numbers and between the sign and the number when testing with (abs|rel) tol
. There are two reasons why:
- The spacing of matrices is unpredictable. You could have
[1.0 0.0] [1.0 0.0]
vs
[0.999999 0.0] [ 1.0 0.0]
Note the added space between [
and 1.0
.
- Complex numbers or polynomials
1.0 + 1e-16*I
vs
1.0 - 1e-16*I
Here you want to compare + 1e-16
and - 1e-16
with the sign for your tolerance computations.
After some consideration, I decided it actually makes a lot of sense to use interval arithmetic for doctest tolerances, this is implemented on this ticket.
Change History (21)
comment:1 Changed 8 years ago by
- Branch set to u/jdemeyer/ticket/16889
- Created changed from 08/27/14 14:43:50 to 08/27/14 14:43:50
- Modified changed from 08/27/14 14:43:50 to 08/27/14 14:43:50
comment:2 Changed 8 years ago by
- Commit set to 46301acf040394a5660c7a9a7262116cfc721804
comment:3 Changed 8 years ago by
- Commit changed from 46301acf040394a5660c7a9a7262116cfc721804 to 291a442c8b006a8224e76cc68df7ad9708333c96
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
291a442 | Doctest tolerance: allow spaces
|
comment:4 Changed 8 years ago by
- Cc zimmerma added
Paul Zimmermann: this needs a minor change to a French book doctest. When using # tol
, you may no longer split doctest results like
1.5 - 2.3
The minus sign may not be on its own line, you need to write
1.5 - 2.3
comment:5 Changed 8 years ago by
- Description modified (diff)
- Status changed from new to needs_review
comment:6 follow-up: ↓ 7 Changed 8 years ago by
in the source of the french book, I've changed to:
[0.347521510119 + 0.566550553398*I, 0.347521510119 - 0.566550553398*I, 0.345023776962 + 0.439908702386*I, 0.345023776962 - 0.439908702386*I, -0.517257614325 + 0.512958206789*I, -0.517257614325 - 0.512958206789*I, -1.36699716455, -9.98357818097]
which uses the same number of lines, and should not make any overfull hbox. Is that ok?
Paul
comment:7 in reply to: ↑ 6 Changed 8 years ago by
Replying to zimmerma:
in the source of the french book, I've changed to:
[0.347521510119 + 0.566550553398*I, 0.347521510119 - 0.566550553398*I, 0.345023776962 + 0.439908702386*I, 0.345023776962 - 0.439908702386*I, -0.517257614325 + 0.512958206789*I, -0.517257614325 - 0.512958206789*I, -1.36699716455, -9.98357818097]which uses the same number of lines, and should not make any overfull hbox. Is that ok?
Yes, that would be good. However, note that Sage itself actually formats the results as in my patch, with one value per line.
comment:8 Changed 8 years ago by
ok, however in our book we allow ourselves not to follow 100% the Sage output format, up to spaces and line breaks, to save some vertical space.
Paul
comment:9 Changed 8 years ago by
Fun fact: this ticket relies on functionality added in 2006:
-
src/sage/ext/mpfr.pyx
commit 04043acc648e0bca14725f147b2040e204ae1f9b Author: William Stein <wstein@ucsd.edu> Date: Mon May 29 01:10:56 2006 +0000 [project @ mpfr -- can coerce in numbers ignoring space, e.g. RR('1 E7') now works (which is what pari outputs!)] diff --git a/src/sage/ext/mpfr.pyx b/src/sage/ext/mpfr.pyx index 8f92208..afc9b72 100644
a b cdef class RealNumber(element.RingElement): 602 602 d = parent(x.denominator()) 603 603 mpfr_div(self.value, n.value, d.value, parent.rnd) 604 604 else: 605 s = str(x) 605 s = str(x).replace(' ','') 606 606 if mpfr_set_str(self.value, s, base, parent.rnd): 607 607 if s == 'NaN' or s == '@NaN@': 608 608 mpfr_set_nan(self.value)
comment:10 Changed 8 years ago by
- Description modified (diff)
comment:11 Changed 8 years ago by
- Commit changed from 291a442c8b006a8224e76cc68df7ad9708333c96 to dde3a3a13ad9d78afe014743d31b766e0ced2af8
Branch pushed to git repo; I updated commit sha1. New commits:
dde3a3a | Use intervals for doctest tolerances
|
comment:12 Changed 8 years ago by
- Description modified (diff)
- Summary changed from Doctest tolerance: allow spaces to Doctest tolerance: allow spaces and use intervals
comment:13 Changed 8 years ago by
- Commit changed from dde3a3a13ad9d78afe014743d31b766e0ced2af8 to 59b511ea9efef56e1e9226216ff67212575e4cde
Branch pushed to git repo; I updated commit sha1. New commits:
59b511e | Add tolerance test file
|
comment:14 Changed 8 years ago by
This looks good to me. How about renaming the tolerance.rst
to must_fail.rst
or so and have an explanatory sentence about what is going on at the top. I fear that somebody will sooner or later "fix" the failing doctests if its not clear ;-)
comment:15 Changed 8 years ago by
- Commit changed from 59b511ea9efef56e1e9226216ff67212575e4cde to 1f858c7511d147e2f93dd0fd4e160fed8ad4f234
Branch pushed to git repo; I updated commit sha1. New commits:
1f858c7 | Add comment that we test failing tests
|
comment:16 Changed 8 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
comment:17 Changed 8 years ago by
- Status changed from positive_review to needs_work
Doctests fail because the line numbers are shifted, e.g. http://build.sagemath.org/sage/builders/%20%20fast%20Volker%20Desktop%20%28Fedora%2020%20x86_64%29%20incremental/builds/416/steps/shell_4/logs/stdio
comment:18 Changed 8 years ago by
- Branch changed from u/jdemeyer/ticket/16889 to u/vbraun/ticket/16889
comment:19 Changed 8 years ago by
- Modified changed from 09/01/14 21:44:18 to 09/01/14 21:44:18
- Status changed from needs_work to positive_review
comment:20 Changed 8 years ago by
- Branch changed from u/vbraun/ticket/16889 to 1f858c7511d147e2f93dd0fd4e160fed8ad4f234
- Resolution set to fixed
- Status changed from positive_review to closed
comment:21 Changed 8 years ago by
- Merged in set to sage-6.4.beta3
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Doctest tolerance: allow spaces