Opened 10 years ago
Closed 8 years ago
#8720 closed defect (fixed)
CC and CDF do not display numeric 0
Reported by: | jason | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | basic arithmetic | Keywords: | |
Cc: | zimmerma, mhansen | Merged in: | sage-5.0.beta14 |
Authors: | Jason Grout, Mike Hansen, Paul Zimmermann | Reviewers: | Paul Zimmermann, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Look at the inconsistency:
sage: RR(0) 0.000000000000000 sage: RDF(0) 0.0
versus
sage: CDF(0) 0 sage: CC(0) 0
Apply trac-8720-printing-complex-zero.patch, trac_8720-doctests.patch, trac_8720-doctests-2.patch and trac_8720-doctests-3.patch.
Attachments (4)
Change History (34)
comment:1 Changed 10 years ago by
- Cc zimmerma added
comment:2 Changed 10 years ago by
- Status changed from new to needs_work
since I was asked to review this ticket, I first notice that some doctests do not pass any more, for example:
sage -t gsl/dft.py ********************************************************************** File "/usr/local/sage-4.3.5-core2/devel/sage-8720/sage/gsl/dft.py", line 528: sage: t = s.fft(); t Expected: Indexed sequence: [5.00000000000000, 0, 0, 0, 0] indexed by [0, 1, 2, 3, 4] Got: Indexed sequence: [5.00000000000000, 0.000000000000000, 0.000000000000000, \ 0.000000000000000, 0.000000000000000] indexed by [0, 1, 2, 3, 4]
thus some more work is needed. Please check all doctests still pass.
comment:3 Changed 10 years ago by
Thanks for looking at it. When I ran the doctests, a number of them failed, so I posted to sage-devel and kept this ticket as "needs work". I will set it to "needs review" when I've taken care of the failing doctests.
Thanks again.
comment:4 Changed 8 years ago by
I'm going to run the tests shortly and produce an doctest fixing patch.
comment:5 Changed 8 years ago by
- Cc mhansen added
comment:6 Changed 8 years ago by
- Status changed from needs_work to needs_review
I've attached a patch which should fix all of the doctests in 4.8.alpha4.
comment:7 Changed 8 years ago by
note that a consequence of that ticket is that coefficients 0 that were not displayed in Taylor series are now displayed, for example:
sage: E = EllipticCurve('37a') sage: L = E.lseries().dokchitser() sage: L.taylor_series(1,4) 0.0000000000000 + 0.305999773834052*z + 0.186547797268162*z^2 - 0.136791463097188*z^3 + O(z^4)
(Before this ticket, the leading 0.000000000000000
was not printed.)
I find it good, since those 0.0 values can come from numerical cancellation.
Paul
comment:8 follow-up: ↓ 9 Changed 8 years ago by
- Reviewers set to Paul Zimmermann
all tests work on top of Sage 4.7.2, I give a positive review.
Paul
comment:9 in reply to: ↑ 8 Changed 8 years ago by
- Status changed from needs_review to positive_review
all tests work on top of Sage 4.7.2, I give a positive review.
Just doing the radio button for this. Nice teamwork on this!
comment:10 Changed 8 years ago by
- Status changed from positive_review to needs_work
A few more doctests need to be fixed:
sage -t -force_lib devel/sage/sage/matrix/matrix2.pyx ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha6/devel/sage-main/sage/matrix/matrix2.pyx", line 7987: sage: M.round(6).zero_at(10^-6) Expected: [ -1.528503 0 0] [ 0.459974 - 0.40061*I -1.741233 0] [-0.934304 + 0.148868*I 0.54833 + 0.073202*I -0.550725] Got: [ -1.528503 0.0 0.0] [ 0.459974 - 0.40061*I -1.741233 0.0] [-0.934304 + 0.148868*I 0.54833 + 0.073202*I -0.550725] ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha6/devel/sage-main/sage/matrix/matrix2.pyx", line 7991: sage: (A - M*G).zero_at(10^-12) Expected: [0 0 0] [0 0 0] [0 0 0] Got: [0.0 0.0 0.0] [0.0 0.0 0.0] [0.0 0.0 0.0] ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha6/devel/sage-main/sage/matrix/matrix2.pyx", line 7995: sage: (G*G.conjugate().transpose()).zero_at(10^-12) Expected: [1.0 0 0] [ 0 1.0 0] [ 0 0 1.0] Got: [1.0 0.0 0.0] [0.0 1.0 0.0] [0.0 0.0 1.0] **********************************************************************
comment:11 follow-up: ↓ 12 Changed 8 years ago by
sorry, but there is no such doctest in 4.7.2. There must be some interaction with some other patch which you included. My positive review was valid for 4.7.2 only.
Paul
comment:12 in reply to: ↑ 11 Changed 8 years ago by
Replying to zimmerma:
sorry, but there is no such doctest in 4.7.2. There must be some interaction with some other patch which you included. My positive review was valid for 4.7.2 only.
Probably Jeroen is referring to doctests in the latest alpha, on which (for better or for worse) patches need to be based on.
comment:13 Changed 8 years ago by
* bump *
comment:14 Changed 8 years ago by
- Status changed from needs_work to needs_review
I posted a patch fixing these problems.
comment:15 Changed 8 years ago by
- Description modified (diff)
Note that the second patch applies with a little fuzz.
$ hg qpush applying trac_8720-doctests.patch patching file sage/matrix/matrix_double_dense.pyx Hunk #1 succeeded at 189 with fuzz 2 (offset 9 lines). now at: trac_8720-doctests.patch
I'll run tests and check. I should have set sage -b
to do parallel building
comment:16 Changed 8 years ago by
{{[
Doctesting 1 file using 1 thread sage -t devel/sage-main/sage/matrix/matrix_double_dense.pyx
[19.7 s]
}}}
Now running full long doctests.
comment:17 Changed 8 years ago by
- Reviewers changed from Paul Zimmermann to Paul Zimmermann, Karl-Dieter Crisman
- Status changed from needs_review to needs_work
$ ../../sage -tp 4 -long sage/lfunctions/lcalc.py Global iterations: 1 File iterations: 1 Using long cached timings to run longest doctests first. Doctesting 1 file using 1 thread sage -t -long devel/sage-main/sage/lfunctions/lcalc.py *** Warning: new stack size = 1028976 (0.981 Mbytes). *** Warning: new stack size = 1003360 (0.957 Mbytes). *** Warning: new stack size = 1001472 (0.955 Mbytes). ********************************************************************** File "/Users/.../sage-5.0.beta2/devel/sage-main/sage/lfunctions/lcalc.py", line 229: sage: E.lseries().values_along_line(0.5, 3, 5) Expected: [(0.000000000, 0.209951303), (0.500000000, -2.92081722e-16), (1.00000000, 0.133768433), (1.50000000, 0.360092864), (2.00000000, 0.552975867)] Got: [(0.000000000, 0.209951303), (0.500000000, -2.92081723e-16), (1.00000000, 0.133768433), (1.50000000, 0.360092864), (2.00000000, 0.552975867)] ********************************************************************** 1 items had failures: 1 of 7 in __main__.example_5 ***Test Failed*** 1 failures. For whitespace errors, see the file /Users/.../.sage/tmp/new_host_2.home-37994/lcalc_37997.py [3.0 s]
The difference was in the e-16 one. I think there is a reason that one used to be -...e-16
in the old doctest. I'm going to ignore the stack size warnings, since they don't seem to have affected the outcome, though I'm sure they're not ideal.
All else is fine!
comment:18 follow-up: ↓ 19 Changed 8 years ago by
Can you double-check to make sure that test was passing before the patches? It seems odd to me that error would have been triggered by this patch.
comment:19 in reply to: ↑ 18 Changed 8 years ago by
Can you double-check to make sure that test was passing before the patches? It seems odd to me that error would have been triggered by this patch.
Notice that the test now fails because we used to ignore all digits before the exponent, not because of the stack thing - the doc even points out that warnings might happen, it's no big deal for now.
Sometimes warnings are printed (by lcalc) when this command is run:: sage: E = EllipticCurve('389a') sage: E.lseries().values_along_line(0.5, 3, 5) [(0, 0.209951303), (0.500000000, -...e-16), (1.00000000, 0.133768433), (1.50000000, 0.360092864), (2.00000000, 0.552975867)]
Here is 5.0.beta1:
$ ./sage -t -long devel/sage/sage/lfunctions/lcalc.py sage -t -long "devel/sage/sage/lfunctions/lcalc.py" [20.4 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 20.5 seconds <snip> sage: sage: E = EllipticCurve('389a') sage: sage: E.lseries().values_along_line(0.5, 3, 5) *** Warning: new stack size = 1003360 (0.957 Mbytes). [(0, 0.209951303), (0.500000000, -2.92081723e-16), (1.00000000, 0.133768433), (1.50000000, 0.360092864), (2.00000000, 0.552975867)]
so we can see that all that has to be done is go back to the previous doctest for that one number (not even the whole list).
comment:20 Changed 8 years ago by
Ah, I understand. Mike made this test much more rigid, and that is what is failing here. Yeah, I agree with your conclusion, Karl-Dieter.
comment:21 Changed 8 years ago by
- Status changed from needs_work to needs_review
I've gone ahead and fixed that doctest.
comment:22 Changed 8 years ago by
- Status changed from needs_review to needs_work
- Work issues set to rebase to current beta
I'm afraid that, according to the patchbot, this new patch works on latest release (4.8) but fails a doctest on 5.0.beta3, and doesn't even apply to 5.0.beta11.
Changed 8 years ago by
Changed 8 years ago by
Changed 8 years ago by
comment:23 Changed 8 years ago by
- Status changed from needs_work to needs_review
- Work issues rebase to current beta deleted
I've rebased the files on beta11.
comment:24 Changed 8 years ago by
- Status changed from needs_review to needs_work
- Work issues set to fix doctests
The beta11 patchbot brings up some doctest failures:
The following tests failed: sage -t -force_lib devel/sage-8720/sage/plot/hyperbolic_triangle.py # 2 doctests failed sage -t -force_lib devel/sage-8720/sage/plot/hyperbolic_arc.py # 1 doctests failed sage -t -force_lib devel/sage-8720/sage/matrix/matrix_space.py # 1 doctests failed sage -t -force_lib devel/sage-8720/sage/matrix/constructor.py # 1 doctests failed
comment:25 Changed 8 years ago by
- Status changed from needs_work to needs_review
I have attached a patch for sage-5.0.beta11, which makes the 4 doctests from comment 24 work. Note that the change in matrix/constructor.py
seems to indicate it
was not working properly before this ticket. Please review.
Paul
comment:26 Changed 8 years ago by
- Description modified (diff)
changed description for the patchbot.
Paul
comment:27 Changed 8 years ago by
Seems to work fine on these files. Running long doctests now but I don't anticipate any more problems.
As to constructor.py, the -0.1
was removed in the first doctest patch, so the problem was in the test, not the behavior.
comment:28 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:29 Changed 8 years ago by
- Work issues fix doctests deleted
comment:30 Changed 8 years ago by
- Merged in set to sage-5.0.beta14
- Resolution set to fixed
- Status changed from positive_review to closed
I think CDF and CC should display the same output as RDF and RR, respectively.
CCing zimmerma since he may be interested in reviewing this.