Opened 10 years ago

Closed 10 years ago

#14448 closed defect (fixed)

Bad sign in sign_mantissa_exponent() methods

Reported by: Thierry Monteil Owned by: Alex Ghitza
Priority: major Milestone: sage-5.10
Component: basic arithmetic Keywords: mpfr, RDF
Cc: Jason Grout, Paul Zimmermann Merged in: sage-5.10.beta0
Authors: Thierry Monteil Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Travis Scrimshaw)

The .sign_mantissa_exponent() methods of the RealNumber and RealDoubleElement classes give a negative mantissa when the number is negative, which does not corresponds to the behaviour described in the documentation : http://www.sagemath.org/doc/reference/rings_numerical/sage/rings/real_mpfr.html http://www.sagemath.org/doc/reference/rings_numerical/sage/rings/real_double.html

We propose here to fix it.

By the way, the variable name 'mantissa' sometimes (always?) appears in the source with the meaning of a signed mantissa. I would suggest to renamed it signed_mantissa or s_mantissa to ease code reading (though i do not feel able to detect which one is signed or not along the code). See for example the .exact_rational() method in real_mpfr.pyx.


Apply: trac_14448_sign_of_mantissa-tm.2.patch

Attachments (2)

trac_14448_sign_of_mantissa-tm.patch (2.7 KB) - added by Thierry Monteil 10 years ago.
Tested on sage 5.9.beta5
trac_14448_sign_of_mantissa-tm.2.patch (2.8 KB) - added by Thierry Monteil 10 years ago.
Patch updated so that it quotes this trac ticket

Download all attachments as: .zip

Change History (9)

Changed 10 years ago by Thierry Monteil

Tested on sage 5.9.beta5

comment:1 Changed 10 years ago by Thierry Monteil

Cc: Jason Grout added
Description: modified (diff)
Keywords: RDF added; float removed

comment:2 Changed 10 years ago by Travis Scrimshaw

I'm guessing this is ready for review?

Looks good, but one small thing, could you change line 2930 in real_mpfr.pyx to:

The mantissa is always a nonnegative number (see :trac:`14448`)::

or something to this extent, noting the problem was fixed in this ticket?

Thanks,
Travis

comment:3 Changed 10 years ago by Jeroen Demeyer

I think it's a bug that the mantissa of 1 and -1 are different.

comment:4 Changed 10 years ago by Paul Zimmermann

Cc: Paul Zimmermann added

Changed 10 years ago by Thierry Monteil

Patch updated so that it quotes this trac ticket

comment:5 Changed 10 years ago by Thierry Monteil

Status: newneeds_review

I added the reference to this trac ticket. I also put this ticket under review.

comment:6 Changed 10 years ago by Travis Scrimshaw

Authors: Thierry Monteil
Description: modified (diff)
Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

Looks good to me.

comment:7 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.10.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.