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

Paticular decimal mod operation wrongly output NaN. #45523

Closed
ocean-city mannequin opened this issue Sep 20, 2007 · 13 comments
Closed

Paticular decimal mod operation wrongly output NaN. #45523

ocean-city mannequin opened this issue Sep 20, 2007 · 13 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ocean-city
Copy link
Mannequin

ocean-city mannequin commented Sep 20, 2007

BPO 1182
Nosy @rhettinger, @facundobatista, @mdickinson

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/facundobatista'
closed_at = <Date 2008-01-08.16:22:33.099>
created_at = <Date 2007-09-20.16:26:58.518>
labels = ['type-bug', 'library']
title = 'Paticular decimal mod operation wrongly output NaN.'
updated_at = <Date 2008-01-08.23:17:21.557>
user = 'https://bugs.python.org/ocean-city'

bugs.python.org fields:

activity = <Date 2008-01-08.23:17:21.557>
actor = 'mark.dickinson'
assignee = 'facundobatista'
closed = True
closed_date = <Date 2008-01-08.16:22:33.099>
closer = 'facundobatista'
components = ['Library (Lib)']
creation = <Date 2007-09-20.16:26:58.518>
creator = 'ocean-city'
dependencies = []
files = []
hgrepos = []
issue_num = 1182
keywords = []
message_count = 13.0
messages = ['56056', '56097', '56114', '56115', '59457', '59488', '59489', '59541', '59555', '59556', '59565', '59569', '59573']
nosy_count = 4.0
nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'ocean-city']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1182'
versions = ['Python 2.5']

@ocean-city
Copy link
Mannequin Author

ocean-city mannequin commented Sep 20, 2007

Following code illegally print "NaN" on Python2.5.

from decimal import *
d1 = Decimal("23.08589694291355371979265447")
d2 = Decimal("2.302585092994045640179914546844")
print d1 % d2

Python2.6(trunk) print 0.06004601297309731799350900156

@ocean-city ocean-city mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 20, 2007
@ocean-city
Copy link
Mannequin Author

ocean-city mannequin commented Sep 23, 2007

I tracked down, and I noticed following code was invoked.

Lib/decimal.py (release-maint25 Decimal#_rescale)

1912: if watchexp and digits > context.prec:
1913: return context._raise_error(InvalidOperation, 'Rescale > prec')

from decimal import *
d = Decimal("23.08589694291355371979265447")
print d % Decimal("2.302585092994045640179914546844") # NaN
print Decimal("0.060046012973097317993509001560")._rescale(-30) # error

Length of decimal seems to be important, so I changed length and it
seemed working.

print d % Decimal("2.302585092994045640179914547") #
0.060046012973097317993509000

Maybe is this intended behavior? Still I feel 2.6's behavior is less
suprising though...

@mdickinson
Copy link
Member

There's a bug on line 1341 of decimal.py. That line currently reads:

otherside = otherside._rescale(exp, context=context)

It should read:

otherside = otherside._rescale(exp, context=context, watchexp=0)

@mdickinson
Copy link
Member

I should have said that the bug I mentioned above is just one of a
number of bugs (mostly in division, addition and square root) that have
been corrected in the trunk. Some of these fixes should probably be
backported. But the decimal module has also had significant new
functionality added since Python 2.5, which is going to make sorting out
which pieces to backport tricky.

Actually, I guess it's possible to argue that the entire new decimal.py
module should be backported for inclusion in Python 2.5.2: the new
functionality was added to comply with the IBM Decimal Arithmetic
specification, and the comments in the decimal module explicitly say
that non-compliance with an update specification should be regarded as a
bug. So almost all the changes are bugfixes, in some sense...

Facundo, what do you think?

@facundobatista
Copy link
Member

Mmm... I thought this would be a clean backport... but no.

If we just copy the files to 2.5, we get two failures running the tests.

  • test_hash_method (DecimalUsabilityTest): This is because of the
    changes we made to the hash builtin in the trunk, and we should avoid
    this test if version < 2.6.

  • test_normalize: Exception "Clamped" raised on line nrmx218. I don't
    have a clue why this one fails.

Mark, could you please take a look at it?

Thank you!

@mdickinson
Copy link
Member

You need to remove the old files in decimaltestdata before copying the
new ones across: nrmx218 is an old, and buggy, testcase; at some point
Mike Cowlishaw renamed normalize.decTest to reduce.decTest. He also
renamed the operation from normalize to reduce, but since this name
change hasn't made it into the most recent version of the specification
it's stayed as normalize in the Python source for now. So it looks like
you ended up with an old version of normalize.decTest in addition to all
the new decTest files.

Note that redx218 in reduce.decTest is identical to nrmx218, except that
it specifies that Clamped *should* be raised.

For the hash method, I think it's safe to leave the old Python 2.5
__hash__ exactly as it is, but backport everything else. This means
that hash will still be slow for large Decimals in Python 2.5 (i.e., we
won't be able to backport the fix for bpo-1770416 in Python 2.5), but
at least it'll be correct.
If we backport the new __hash__ without also backporting the
corresponding core change to the long __hash__ then we'll be left with a
buggy __hash__. The new tests for __hash__ are still valid, and I think
they shouldn't be skipped in the backported version.

And I definitely don't want to suggest backporting the long.__hash__
change---that just seems to be asking for trouble.

@mdickinson
Copy link
Member

P.S. I've just noticed that both versions of __hash__ are buggy: the
hash value of a Decimal depends on the current context. I'll open a new
bug report.

@facundobatista
Copy link
Member

Decimal was backported to Py2.5, commited in r59859.

@mdickinson
Copy link
Member

Unfortunately, I think this backport still breaks hash:

bernoulli:~/python_source/release25-maint dickinsm$ ./python.exe
Python 2.5.2a0 (release25-maint:59859M, Jan  8 2008, 11:54:53) 
[GCC 4.0.1 (Apple Computer, Inc. build 5370)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from decimal import *
>>> x = Decimal("1.634E100")
>>> hash(x) == hash(int(x))
False

Do we really want to go from a slow-but-working Decimal.__hash__ in Python 2.5.1 to a fast-but-
broken hash in Python 2.5.2?

I can fix this (it's a 1-line change), and reinstate the extra hash tests, if you like. Or I can
post a patch if you prefer.

@facundobatista
Copy link
Member

Fix it, please.

@mdickinson
Copy link
Member

hash fixed in revision 59863.

@rhettinger
Copy link
Contributor

If this something missing from Colishaw's test suite, you should submit
the result to him so they can include it in the next update.

@mdickinson
Copy link
Member

I don't think anything's missing from Cowlishaw's test-suite. An old,
buggy test (nrmx218) was both renamed (to redx218) and fixed by Cowlishaw.
I think Facundo ended up with both tests---so naturally the old, broken
test failed.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants