Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CC and CDF do not display numeric 0 #8720

Closed
jasongrout opened this issue Apr 20, 2010 · 41 comments
Closed

CC and CDF do not display numeric 0 #8720

jasongrout opened this issue Apr 20, 2010 · 41 comments

Comments

@jasongrout
Copy link
Member

Look at the inconsistency:

sage: RR(0)
0.000000000000000
sage: RDF(0)
0.0

versus

sage: CDF(0)
0
sage: CC(0)
0

Apply attachment: trac-8720-printing-complex-zero.patch, attachment: trac_8720-doctests.patch, attachment: trac_8720-doctests-2.patch
and attachment: trac_8720-doctests-3.patch.

CC: @zimmermann6 @mwhansen

Component: basic arithmetic

Author: Jason Grout, Mike Hansen, Paul Zimmermann

Reviewer: Paul Zimmermann, Karl-Dieter Crisman

Merged: sage-5.0.beta14

Issue created by migration from https://trac.sagemath.org/ticket/8720

@jasongrout
Copy link
Member Author

Author: Jason Grout

@jasongrout
Copy link
Member Author

comment:1

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.

@zimmermann6
Copy link

comment:2

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.

@jasongrout
Copy link
Member Author

comment:3

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.

@mwhansen
Copy link
Contributor

comment:4

I'm going to run the tests shortly and produce an doctest fixing patch.

@mwhansen
Copy link
Contributor

comment:6

I've attached a patch which should fix all of the doctests in 4.8.alpha4.

@zimmermann6
Copy link

comment:7

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

@zimmermann6
Copy link

Reviewer: Paul Zimmermann

@zimmermann6
Copy link

Changed author from Jason Grout to Jason Grout, Mike Hansen

@zimmermann6
Copy link

comment:8

all tests work on top of Sage 4.7.2, I give a positive review.

Paul

@kcrisman
Copy link
Member

comment:9

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!

@jdemeyer
Copy link

comment:10

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]
**********************************************************************

@zimmermann6
Copy link

comment:11

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

@kcrisman
Copy link
Member

comment:12

Replying to @zimmermann6:

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.

@jdemeyer
Copy link

jdemeyer commented Feb 7, 2012

comment:13

*** bump ***

@mwhansen
Copy link
Contributor

mwhansen commented Feb 8, 2012

comment:14

I posted a patch fixing these problems.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 29, 2012

Work Issues: rebase to current beta

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 29, 2012

comment:22

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.

@loefflerd loefflerd mannequin added s: needs work and removed s: needs review labels Mar 29, 2012
@mwhansen
Copy link
Contributor

Attachment: trac-8720-printing-complex-zero.patch.gz

@mwhansen
Copy link
Contributor

Attachment: trac_8720-doctests.patch.gz

Attachment: trac_8720-doctests-2.patch.gz

@mwhansen
Copy link
Contributor

comment:23

I've rebased the files on beta11.

@mwhansen
Copy link
Contributor

Changed work issues from rebase to current beta to none

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 30, 2012

Work Issues: fix doctests

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 30, 2012

comment:24

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

@loefflerd loefflerd mannequin added s: needs work and removed s: needs review labels Mar 30, 2012
@zimmermann6
Copy link

Attachment: trac_8720-doctests-3.patch.gz

apply this patch on top of the other three

@zimmermann6
Copy link

Changed author from Jason Grout, Mike Hansen to Jason Grout, Mike Hansen, Paul Zimmermann

@zimmermann6
Copy link

comment:25

I have attached a patch for sage-5.0.beta11, which makes the 4 doctests from comment [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

@zimmermann6
Copy link

comment:26

changed description for the patchbot.

Paul

@zimmermann6

This comment has been minimized.

@kcrisman
Copy link
Member

kcrisman commented Apr 3, 2012

comment:27

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.

@kcrisman
Copy link
Member

kcrisman commented Apr 3, 2012

Changed work issues from fix doctests to none

@jdemeyer
Copy link

Merged: sage-5.0.beta14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants