Patches item #1741308, was opened at 2007-06-22 06:54 Message generated for change (Comment added) made by marketdickinson You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1741308&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Library (Lib) Group: Python 2.6 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Mark Dickinson (marketdickinson) Assigned to: Facundo Batista (facundobatista) Summary: Fix Decimal.sqrt bugs described in #1725899 Initial Comment: This patch fixes a number of (yet-to-be-confirmed-as-) bugs in Decimal.sqrt(); see bug report 1725899 for details of these. As a side benefit the modified version of Decimal.sqrt() runs significantly faster than the original, since it's based on integer arithmetic instead of carrying out Newton's method entirely in decimal; on my iBook G4 the speedup is between 20 and 25 times with the default precision. ---------------------------------------------------------------------- >Comment By: Mark Dickinson (marketdickinson) Date: 2007-07-18 03:37 Message: Logged In: YES user_id=703403 Originator: YES Final(?) version of the patch is attached. The extra testcases for sqrt have been vetted, corrected (see below) and slightly expanded by Mike Cowlishaw, and are, I believe, going to be included in the next update on his website. The file squareroot_extra.decTest included in the patch is exactly the file I received back from Cowlishaw, with no changes. The code and comments have been cleaned up a little. The patch also fixes another bug in the _fixexponents method: in the following, the Underflow flag should not be raised. Python 2.5.1 (r251:54863, May 10 2007, 20:59:25) [GCC 4.0.1 (Apple Computer, Inc. build 5367)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> from decimal import * >>> c = getcontext() >>> c.Emin = -9 >>> c.Emax = 9 >>> Decimal(2).sqrt() # Inexact but not Subnormal Decimal("1.414213562373095048801688724") >>> +Decimal("100E-38") # Subnormal but not Inexact Decimal("1E-36") >>> c Context(prec=28, rounding=ROUND_HALF_EVEN, Emin=-9, Emax=9, capitals=1, flags=[Rounded, Underflow, Subnormal, Inexact], traps=[DivisionByZero, Overflow, InvalidOperation]) There's still one outstanding issue: Cowlishaw says that for a general arithmetic operation, underflow of a nonzero result to zero should result in the Clamped flag being raised; I believe that the current testcases from Cowlishaw's website reflect this, while the older ones in the decimaltestdata directory don't. (This is the source of the only corrections to the previous test cases.) Currently sqrt() does raise Clamped on underflow to 0, but none of the other arithmetic operations do, which is inconsistent. I'd be happy to fix this, but I think discussion is needed first: should sqrt be `fixed' to not raise Clamped here (this would be trivial to do); should the other arithmetic operations be fixed to raise Clamped on underflow to 0 (also trivial, except that it would require updating many of the testcases), or should things be left as they are? Any suggestions? (By the way, I also have a working Decimal.log, if anyone's interested...) Mark Mark File Added: decimal_sqrt_3.patch ---------------------------------------------------------------------- Comment By: Mark Dickinson (marketdickinson) Date: 2007-07-02 14:34 Message: Logged In: YES user_id=703403 Originator: YES File Added: decimal_sqrt_2.patch ---------------------------------------------------------------------- Comment By: Mark Dickinson (marketdickinson) Date: 2007-07-02 14:33 Message: Logged In: YES user_id=703403 Originator: YES Here's an updated patch, with: -- several hundred extra testcases, in a new file squareroot_extra.decTest -- fixes for two bugs in the previous patch; one where the context rounding mode was occasionally used instead of ROUND-HALF-EVEN, and one involving underflow to 0. -- a fix for the following problem in Decimal._fixexponents(): (should I submit a separate patch for this instead?) >>> from decimal import * >>> getcontext().Emax = 9 >>> getcontext().Emin = -9 >>> getcontext()._clamp = 1 >>> +Decimal("1E10") Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/opt/local/lib/python2.5/decimal.py", line 935, in __pos__ ans = self._fix(context) File "/opt/local/lib/python2.5/decimal.py", line 1530, in _fix ans = self._fixexponents(context) File "/opt/local/lib/python2.5/decimal.py", line 1565, in _fixexponents ans = ans._rescale(Etop, context=context) File "/opt/local/lib/python2.5/decimal.py", line 1913, in _rescale return context._raise_error(InvalidOperation, 'Rescale > prec') File "/opt/local/lib/python2.5/decimal.py", line 2325, in _raise_error raise error, explanation decimal.InvalidOperation: Rescale > prec I've been in contact with Cowlishaw; he's agreed that it looks as though the C reference implementation needs a few changes, but hasn't had a chance to look at things properly yet; I've sent him the extra testcases above as well. Please let me know if there's anything else I can do to help. Mark ---------------------------------------------------------------------- Comment By: Facundo Batista (facundobatista) Date: 2007-06-22 12:29 Message: Logged In: YES user_id=752496 Originator: NO Yes, I'll handle this and the mentioned bug. Thanks. ---------------------------------------------------------------------- Comment By: Neal Norwitz (nnorwitz) Date: 2007-06-22 07:09 Message: Logged In: YES user_id=33168 Originator: NO Facundo, could you take a look? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1741308&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches