Proposal for new is_approx() behaviour

[paultcochrane]

While documenting the is_approx() test function behaviour I was confused by a (potentially only perceived by me) difference between the specs and the actual behaviour. Namely, the specs say that is_approx will return True when two numbers are within 1e-5 of one another. This implies to me something like this:

return ($got - $expected).abs <= 1e-5;

However, the implementation allows large numbers to be "approximately equal" if 6 significant digits (i.e. not "within 1e-5") are the same. In other words:

my $speed_of_light = 2.99792458e8;
my $not_quite_sol = 2.997925e8;
is_approx($not_quite_sol, $speed_of_light);   # returns True, however differ by 42

On the other hand, small numbers (e.g. smaller than 1e-6) are always "approximately equal" even if the values are wildly different:

my $plancks_constant = 6.62609657e-34;
my $not_quite_pc = 16.62608e-34;
is_approx($not_quite_pc, $plancks_constant);  # returns True, but *really* shouldn't

The reason that this returns True is that both numbers are smaller than 1e-5.

Due to this confusion, and after comparing this behaviour with other testing frameworks (e.g. http://docs.scipy.org/doc/numpy/reference/generated/numpy.testing.assert_approx_equal.html#numpy.testing.assert_approx_equal), I propose changing the current behaviour of is_approx() to consider two numbers to be approximately equal if they are equal up to a given number of significant digits. This allows is_approx() to behave the same for the "large" and "small" numbers mentioned above and is thus a more general solution.

The proposed changes given in this PR are based upon http://en.wikipedia.org/wiki/Relative_change_and_difference#Formulas and http://c-faq.com/fp/fpequal.html.

The PR includes "tests", which are not intended to be merged, but merely programmatically to show in a more detailed manner what I mean here; both the current and proposed behaviour.

By making this change in behaviour, it was also possible to make is_approx() accept a variable tolerance. Thus it is now possible to write code like this:

is_approx($got, $expected, $tol = 1e-7, $description = "");

I realise this is a large change and could break a lot of code, thus I'd appreciate feedback as to how sensible this change is and whether or not it is appropriate. It would, however, be very good to remove the ambiguity in the current behaviour.

[colomon]

First, let me thank you for taking the point on this! I think it's an important improvement that needs to be done, and you've got a great start here. Here are some random thoughts on the patch:

Why are $got and $expected Mu, but $tol is Num? My first instinct is that $tol should be a Real. Should $got and $expected be Num? Do they work if one or both of them are Complex? Do we want to allow non-Num types to be passed?

Does your code work if $expected is 0?

And should we take advantage of a switch to also switch it to is-approx?

[paultcochrane]

Why are $got and $expected Mu, but $tol is Num?

Good question. Due to "historical reasons" I guess. Actually this is sort of a leftover from the original implementation which uses Mu for $got and $expected. I'd intented to change these to Num but forgot to add this. Perhaps $tol should be Real as this makes sense for real and complex numbers (one has to take the abs anyway).

Do they work if one or both of $got or $expected are Complex?

I don't believe so. This would be a good extension to the code though, thanks for mentioning it!

Do we want to allow non-Num types to be passed?

I don't know directly, however instinctively I would say non-Num types should not be passed and that an appropriate error should be printed.

Does your code work if $expected is 0?

Well caught! The current version ( f904b21) doesn't work with 0. I'll fix this and push as soon as I can.

And should we take advantage of a switch to also switch it to is-approx?

I think this is a very good idea in general. All functions should use the more Perl6-y way of naming variables. Such a change would break a lot of code (Rakudo, the spec tests, roast, moar etc. are all probably affected by such a change) so this needs to be a careful and well discussed before being carried out. At present, I'd say the current change to is_approx shouldn't include renaming the function in order to keep existing code working as well as to keep the Test API consistent.

[jnthn]

If there is an agreement to rename, then a possible strategy would be to introduce is-approx with the new semantics, then deprecate the existing is_approx. That way the migration can be more easily managed, rather than having to try and update everything at once.

[perlpilot]

What jnthn said!

-Scott

On Thu, Feb 19, 2015 at 5:43 AM, Jonathan Worthington <
notifications@github.com> wrote:

If there is an agreement to rename, then a possible strategy would be to
introduce is-approx with the new semantics, then deprecate the existing
is_approx. That way the migration can be more easily managed, rather than
having to try and update everything at once.

—
Reply to this email directly or view it on GitHub
#366 (comment).

[paultcochrane]

Comparison of values around zero

I've added a version of is_approx which handles comparison of values around zero. See the discussion surrounding PEP485 (https://www.python.org/dev/peps/pep-0485/), its implementation: https://github.com/PythonCHB/close_pep/blob/master/is_close.py as well as the assert_allclose behaviour: http://docs.scipy.org/doc/numpy/reference/generated/numpy.testing.assert_allclose.html#numpy.testing.assert_allclose for the reasoning I used in choosing the implementation. What I don't like is the proliferation of multi-subs now used to handle the various calling situations. Advice as how to improve/generalise the code would be greatly appreciated.

Rename is_approx to is-approx

I agree with @jnthn: the rename can take place, however with an appropriate deprecation period. I'll make this change in a separate PR.

Handle Complex, Rat etc.

Still TODO. The discussion surrounding PEP 485 mentioned above describes handling non-floating-point numbers well, and the implementation will very likely follow the advice mentioned in that document.