Issue1053375
Created on 2004-10-24 21:01 by facundobatista, last changed 2004-10-27 06:24 by rhettinger. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| decimal.py | facundobatista, 2004-10-26 02:56 | Modified decimal.py | ||
| decimal.py | facundobatista, 2004-10-27 01:44 | Re-modified decimal.py | ||
| Messages (6) | |||
|---|---|---|---|
| msg47130 - (view) | Author: Facundo Batista (facundobatista) * ![]() |
Date: 2004-10-24 21:01 | |
Raymond: Some minor modifications: max() instead of veryfing which is greater and assigning it, and not passing prec to _fixexponents() (not uses it). Major change: Not use _WorkRep anymore! Changed __add__, __mul__ and __divide__ to support it (including the support functions _normalize() and _adjust_coefficients). And of course, Decimal.__new__ does not support passing _WorkRep anymore, ;). One change that may raise controversial: The syntax to pass a tuple to __new__ now accepts also a number in the second position. This is because in some places in the code the result is just a number, and converting it to a tuple in the code (and not in __new__) made redundant some checks in __new__. All the tests pass ok. The new code shows a 12% speed up in test_decimal.py and 10% in the telco benchmark. . Facundo |
|||
| msg47131 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2004-10-25 00:15 | |
Logged In: YES user_id=80475 It looks like the file did not attach. Please submit a whole decimal.py instead of a diff. Thx, Raymond |
|||
| msg47132 - (view) | Author: Facundo Batista (facundobatista) * ![]() |
Date: 2004-10-26 02:56 | |
Logged In: YES user_id=752496 Oops, don't know what happened. Here's the file complete. . Facundo |
|||
| msg47133 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2004-10-26 04:53 | |
Logged In: YES user_id=80475 I tend to think this should probably not be done. On the plus side, there is the simplification of code (saving approx 80 lines and eliminating an entire class of objects). Also, there is the modest speed-up. On the minus side, it is rather late to be making big changes to the module. More importantly, I'm concerned about inlining _WorkRep's internals. Nick's use of Python ints as a temporary representation is currently confined to _WorkRep. That choice was not the right way to go in the long term. While it saves time for the Python implementation, a C version would go back to the method of shuffling bcd digits directly (giving better big O performance and eliminating an intermodule code dependency). The elimination of prec in the _fixexponents call is okay. Also, the max() call is fine. |
|||
| msg47134 - (view) | Author: Facundo Batista (facundobatista) * ![]() |
Date: 2004-10-27 01:44 | |
Logged In: YES user_id=752496 Commited the previous minor changes. And took another approach: left _WorkRep but with improvements: - Optimized the conversion from Decimal's int to _WorkRep's int (in _WorkRep). - Changed a comparison in __add__ so there's no more need of __cmp__, __abs__, and __neg__ in _WorkRep. - _WorkRep's sign now uses the scheme of Decimal, so I eliminated a lot of changes from (0,1) to (1,None,-1) and viceversa. Now it shows a speedup of 6% in the tests and 4% in the telco benchmark, but I think that the greater win is in the readibility of the code (specially with the signs). I attach the new decimal.py. . Facundo |
|||
| msg47135 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2004-10-27 06:24 | |
Logged In: YES user_id=80475 Nice simplification. Checked in with minor modifications: * reduce() replaced with clearer, faster for-loop (see timings below) * chain of ifs covered distinct cases, so used if-elif-else to avoid unnecessary tests. See Lib/decimal.py 1.30 ----- Timing of for-loop vs reduce() with lambda -------- >>> min(Timer(s1, setup).repeat(9, 1000)) 0.039173469216714996 >>> min(Timer(s1, setup).repeat(9, 100000)) 3.8988694078010013 >>> min(Timer(s2, setup).repeat(9, 100000)) 2.2458371746090293 >>> s1 'cum = reduce(lambda x,y: 10*x+y, val)' >>> s2 '\ncum = 0\nfor dig in val:\n cum = cum * 10 + dig\n' >>> setup '\nval = [9,6,1,2,3,0,2,4,1,5,7,1,5,6,9]\n' |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2004-10-24 21:01:30 | facundobatista | create | |
