Picky floats

[smontanaro]

BPO	1478364
Nosy	@tim-one, @loewis, @smontanaro
Files	picky.diff

^{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: ```python assignee = None closed_at = created_at = labels = ['interpreter-core'] title = 'Picky floats' updated_at = user = 'https://github.com/smontanaro' ``` bugs.python.org fields: ```python activity = actor = 'loewis' assignee = 'none' closed = True closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'skip.montanaro' dependencies = [] files = ['7204'] hgrepos = [] issue_num = 1478364 keywords = ['patch'] message_count = 6.0 messages = ['50121', '50122', '50123', '50124', '50125', '50126'] nosy_count = 3.0 nosy_names = ['tim.peters', 'loewis', 'skip.montanaro'] pr_nums = [] priority = 'normal' resolution = 'rejected' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue1478364' versions = [] ```

[smontanaro]

I came across a bug at work yesterday where I had written:

    if not delta:
        return 0.0

where delta was a floating point number. After a series of calculations piling up round-off error it took on a value on the order of 1e-8. Not zero, but it should have been. The fix was easy enough:

    if abs(delta) < EPSILON:
        return 0.0

for a suitable value of EPSILON.

That got me to thinking... I'm sure I have plenty of other similar mistakes in my code. (Never was much of a numerical analysis guy...) What if there was a picky-float configure option that generated warnings if you compared a float with either another float or an int using "=="? Does that make sense to try for testing purposes? The initial implementation seemed straightforward enough, though I'm sure if this idea merits any serious consideration it will need some more work (at the least this behavior should probably be user-enabled).

[smontanaro]

Logged In: YES user_id=44345

FYI, regarding my original note and the attached patch, I realized that comparison with ints is mostly (always?) okay, so I took that out of the patch. Forgot to amend the wording above though.

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Logged In: YES user_id=21627

I don't think it should be configure-time option. If implemented, it should be a run-time option, e.g. through a interpreter command line option, or even a flag in the sys module.

OTOH, I doubt that it is very useful in practice. How many warnings do you get out of the test suite when this is on?

[smontanaro]

Logged In: YES user_id=44345

Yeah, I don't think configure-time selection is the ultimate choice either. Ideally, I think it's something you might want to enable on a per-module basis, but other than __future__ settings I'm not aware of anything.

I just ran "make test" which runs everything twice. Looking at the output it seems to be complaining about 93 locations. All but 16 of them are in the test suite itself. I've not checked any of them carefully yet. A quick scan suggests most are comparisons with small integer-valued floats, so most are probably innocuous.

I'm not surprised there's not a lot of problem in the standard library though, since it doesn't really do much in the way of numerical algorithms. It might be more instructive to look at some more floating point-intensive code (scipy?, MayaVi?).

I'm less interested in finding specific bugs at this point though. I'm more concerned with deciding whether a suitably constrained implementation can be provided.

(BTW, off-list I got a somewhat related report of lots of float comparison warnings in the Python C code by Intel's C compiler. I've asked him to run it again and file a bug report.)

Skip

[tim-one]

Logged In: YES user_id=31435

The problem with looking "at some more floating point-intensive code (scipy?, MayaVi?)" is that people writing scientific code generally don't use float comparison incorrectly -- wading through false positives isn't all that much fun ;-)

[61337411-43fc-4a9c-b8d5-4060aede66d0]

Since there were some objections to the patch, and apparently no progress in addressing these objections, I'm now rejecting the patch (on its first birthday).