decimal module doesn't respect precedence rules for exceptional conditions

[mdickinson]

BPO	8567
Nosy	@rhettinger, @facundobatista, @mdickinson, @skrah
Files	issue8567.patch issue8567_2.patch issue8567_3.patch

^{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/mdickinson'
closed_at = <Date 2010-05-04.14:39:07.607>
created_at = <Date 2010-04-29.11:10:27.184>
labels = ['type-bug']
title = "decimal module doesn't respect precedence rules for exceptional conditions"
updated_at = <Date 2010-05-04.14:39:07.606>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2010-05-04.14:39:07.606>
actor = 'mark.dickinson'
assignee = 'mark.dickinson'
closed = True
closed_date = <Date 2010-05-04.14:39:07.607>
closer = 'mark.dickinson'
components = []
creation = <Date 2010-04-29.11:10:27.184>
creator = 'mark.dickinson'
dependencies = []
files = ['17163', '17164', '17167']
hgrepos = []
issue_num = 8567
keywords = ['patch']
message_count = 9.0
messages = ['104484', '104494', '104495', '104502', '104503', '104717', '104722', '104731', '104944']
nosy_count = 4.0
nosy_names = ['rhettinger', 'facundobatista', 'mark.dickinson', 'skrah']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue8567'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

[mdickinson]

http://speleotrove.com/decimal/daexcep.html specifies a precedence for decimal exceptional conditions (scroll right to the bottom of the page):

"""The Clamped, Inexact, Rounded, and Subnormal conditions can coincide with each other or with other conditions. In these cases then any trap enabled for another condition takes precedence over (is handled before) all of these, any Subnormal trap takes precedence over Inexact, any Inexact trap takes precedence over Rounded, and any Rounded trap takes precedence over Clamped."""

Currently the decimal module doesn't follow this. For example, the following should raise decimal.Overflow, not decimal.Inexact:

Python 3.2a0 (py3k:80609, Apr 29 2010, 11:46:22)
[GCC 4.2.1 (SUSE Linux)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from decimal import *
>>> getcontext().traps[Inexact] = True
>>> Decimal('1e100').exp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 3002, in exp
    ans = ans._fix(context)
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 1658, in _fix
    context._raise_error(Inexact)
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 3866, in _raise_error
    raise error(explanation)
decimal.Inexact: None

It's also not clear to me exactly which flags should be set in a case like this.

[mdickinson]

Regarding the flags that should be set, the relevant portion of the specification is at the top of the page mentioned above:

"""
For each condition, the corresponding signal in the context is given, along with the defined result. The value of the trap-enabler for each signal in the context determines whether an operation is completed after the condition is detected or whether the condition is trapped and hence not necessarily completed (see IEEE 754 §7 and §8).
"""

Reading between the lines, I think this means that it's okay for an exception resulting from a trap to leave the flag state undefined, at least for those flags that would have been raised by the operation. (But flags that were already set before the operation should remain set; while flags corresponding to the signals that would not have been raised by the operation should not be changed.)

Section 8 if IEEE 754 backs this up to some extent. Python's decimal traps effectively provide what IEEE 754 section 8.3 calls "Immediate transfer associated with a block", a form of "immediate exception handling". Note 2 in that section says:

"""
[...] If the indicated exception is signaled in the associated block, causing execution of the handler block or transfer of control, then the state of the corresponding status flag might not be deterministic.
"""

This doesn't *exactly* apply here, since it's really talking about a sequence of primitive operations rather than a single primitive operation, but it's close. (One might reasonably ask that primitive operations be regarded as atomic, so act as though all relevant flags are set, and only *then* are any relevant traps raised. This would not be a trivial change, however.)

In spite of this indeterminacy, it would still be nice to have clear rules about what Python's decimal module actually does, even if those rules are only for developer consumption.

The simplest (and closest to what's already implemented) would be the following:

• any particular operation (i.e. operands + function) determines uniquely some set of signals; the decimal specification gives us an ordering on those signals.

• the result of the operation should be as if we go through those signals one-by-one in order, and for each signal:

  (a) if the signal is trapped, raise the corresponding exception
  (b) else set the corresponding flag, and move on to the next signal.

Example to clarify these rules: the overflow example above produces the signals [Overflow, Inexact, Rounded] in that order. If 'Overflow' is trapped, the Overflow exception is raised and no flags are set. Otherwise, if 'Inexact' is trapped, the Inexact exception is raised and the 'Overflow' flag is set. Otherwise, if 'Rounded' is trapped, the Rounded exception is raised and the 'Overflow' and 'Inexact' flags will have been set. Otherwise, no exception is raised and all three flags are set.

[mdickinson]

Raising priority: I'd like to get the precedence fix into 2.7.

[skrah]

Mark,

I'm very short of time today, so I hope I don't miss anything that you
wrote.

The model of decNumber (and libmpdec) is to accumulate any status
(flags) that occurs in a function, regardless of whether a signal
is trapped or not.

Only at function exit the traps are checked. If multiple exceptions
could be raised, the order of precedence applies. All status that
has been accumulated in the function remains in the context.

This would be backed up by:

http://speleotrove.com/decimal/damodel.html

==> flags and trap-enablers:

==> "For each of the signals, the corresponding flag is set to 1
when the signal occurs. It is only reset to 0 by explicit
user action."

So, in your example, in cdecimal I get:
>>> from cdecimal import *
>>> getcontext().traps[Inexact] = True
>>> Decimal('1e100').exp()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
cdecimal.Overflow: [<class 'cdecimal.Overflow'>, <class 'cdecimal.Inexact'>]

In the square brackets I list all signals that occurred _and_ are
trapped.

"

[mdickinson]

Thanks, Stefan. The cdecimal and decnumber behaviour sounds like the ideal one to me.

I'm not sure how easy it would be to adopt this behaviour for decimal: it could require some significant rewriting, so we may have to leave decimal as it is and call the flag situation undefined.

I'll take a proper look sometime (soon, I hope); Decimal._fix is the main place that's going to need fixes. The hard part is tracking down all the other places where a signal is raised.

[mdickinson]

Here's a quick fix for Decimal._fix, that just makes sure that it raises exceptions in the appropriate order.

I'll also try to apply the check_precedence methodology included in this patch to every single testcase. I don't think it's reasonable to add this testing to the test-suite, though: test_decimal is slow enough as it is.

[mdickinson]

Better patch, that checks exception precedence for every test case when EXTENDEDERRORTEST is defined. With this patch, I get 11 test failures in test_decimal (down from 50 test failures before the Decimal._fix fix).

Now we just have to track down the remaining wrongly ordered exceptions...

[mdickinson]

Final patch (?!) fixes all the test failures due to exception precedence issues.

[mdickinson]

Precedence fixed in r80753 through r80756.

That still leaves open the problem of what flags should be set; however, we should discuss this in a separate issue.