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

bignum fails to compute log(2) #14908

Open
p5pRT opened this issue Sep 15, 2015 · 8 comments
Open

bignum fails to compute log(2) #14908

p5pRT opened this issue Sep 15, 2015 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 15, 2015

Migrated from rt.perl.org#126061 (status was 'open')

Searchable as RT126061$

@p5pRT
Copy link
Author

p5pRT commented Sep 15, 2015

From ntysdd@gmail.com

Perl version : 5.22.0 / MSWin32-x64-multi-thread

perl -Mbignum -le "print log(2)"
NaN

perl -Mbignum -le "print log(2.1-0.1)"
0.6931471805599453094172321214581765680755

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2015

From @jkeenan

On Tue Sep 15 11​:08​:49 2015, ntysdd@​gmail.com wrote​:

Perl version : 5.22.0 / MSWin32-x64-multi-thread

perl -Mbignum -le "print log(2)"
NaN

And what's even more unfortunate is the fact that this is cited in the documentation as something "to impress the Python crowd." ;-(

From ./dist/bignum/lib/bignum.pm in blead​:

#####
643 =head1 EXAMPLES
644
645 Some cool command line examples to impress the Python crowd ;)
646
...
652 perl -Mbignum -le 'print log(2)'
#####

perl -Mbignum -le "print log(2.1-0.1)"
0.6931471805599453094172321214581765680755

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2015

From @pjacklam

I am aware of this bug, and I already have a patch and a test case.

This bug was introduced when I fixed a different bug, and there was no test
case to reveal that I had introduced a new problem. Fixing bugs in the
spaghetti-code in Math​::Big* and big(int|num|rat) is like whack-a-mole – I
knock down one bug and then a new bug pops up at some unexpected place.

Here is the underlying problem​: The pragmas big(num|int|rat) are supposed
to have the same semantics as core Perl. That is, when they are used, it
should look and feel just like core Perl, but have support for arbitrarily
large numbers. However, behind the scenes, big numbers are handled by
Math​::Big(Int|Rat|Float) which have different semantics than core Perl.
Some effort has been made to differentiate the two, but it is not enough.

As of right now I see two possibilities​:
- Introduce a config option to Math​::Big(Int|Rat|Float) so that, e.g.,
bigint.pm can say "use Math​::BigInt semantics => 'core';" or something like
that. With this option in uses, new() and the mathematical operators will
behave like core Perl.
- Introduce subclasses which overload new() and the mathematical operators,
e.g., Math​::BigInt​::CoreSemantics.

I haven't decided on which is the best. Input - of course including other
options - is highly appreciated. The simplest is probably to do the latter,

Peter

2015-09-16 2​:44 GMT+02​:00 James E Keenan via RT <perlbug-followup@​perl.org>​:

On Tue Sep 15 11​:08​:49 2015, ntysdd@​gmail.com wrote​:

Perl version : 5.22.0 / MSWin32-x64-multi-thread

perl -Mbignum -le "print log(2)"
NaN

And what's even more unfortunate is the fact that this is cited in the
documentation as something "to impress the Python crowd." ;-(

From ./dist/bignum/lib/bignum.pm in blead​:

#####
643 =head1 EXAMPLES
644
645 Some cool command line examples to impress the Python crowd ;)
646
...
652 perl -Mbignum -le 'print log(2)'
#####

perl -Mbignum -le "print log(2.1-0.1)"
0.6931471805599453094172321214581765680755

--
James E Keenan (jkeenan@​cpan.org)

---
via perlbug​: queue​: perl5 status​: new
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126061

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2015

From perl@profvince.com

Le 16/09/2015 10​:03, Peter John Acklam a écrit :

I am aware of this bug, and I already have a patch and a test case.

This bug was introduced when I fixed a different bug, and there was no
test case to reveal that I had introduced a new problem. Fixing bugs in
the spaghetti-code in Math​::Big* and big(int|num|rat) is like
whack-a-mole – I knock down one bug and then a new bug pops up at some
unexpected place.

Here is the underlying problem​: The pragmas big(num|int|rat) are
supposed to have the same semantics as core Perl. That is, when they are
used, it should look and feel just like core Perl, but have support for
arbitrarily large numbers. However, behind the scenes, big numbers are
handled by Math​::Big(Int|Rat|Float) which have different semantics than
core Perl. Some effort has been made to differentiate the two, but it is
not enough.

As of right now I see two possibilities​:
- Introduce a config option to Math​::Big(Int|Rat|Float) so that, e.g.,
bigint.pm <http​://bigint.pm> can say "use Math​::BigInt semantics =>
'core';" or something like that. With this option in uses, new() and the
mathematical operators will behave like core Perl.
- Introduce subclasses which overload new() and the mathematical
operators, e.g., Math​::BigInt​::CoreSemantics.

I haven't decided on which is the best. Input - of course including
other options - is highly appreciated. The simplest is probably to do
the latter,

Peter

Apologies if I'm oversimplifying this, but it just looks like the 'log'
overload callback in Math​::BigInt shouldn't pass $_[1] to the blog()
method. It's always undef for an unary operator, but blog() interprets
it as the base, which causes the NaN.

Vincent

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2015

From @pjacklam

​I'm afraid that is not enough. log() calls blog(), which in turn calls
objectify() to ensure that the two first arguments, the operand and the
base, are objects. If the base is missing or undef, objectify() converts it
to zero, but zero is not a valid base, hence the NaN. It is OK to pass an
undefined base to blog(). It means that blog() should use the base e
(Euler's constant).

One option is to modify objectify() so undefs pass through unmodified, but
I'm not sure if this will have other unfortunate consequences. The tests
pass with this modification, but I know from experience that the test suite
doesn't test everything. One alternative is to modify blog() to handle this
special case where the base is undef. The latter is probably the best. I
can't think of any other operator/method where undef has a special meaning.

I also realize now that the Math-BigInt distribution contains NO tests for
the overloaded functions log, exp, sin, cos, atan2, int, neg, abs, and
sqrt. While creating some tests, I also discovered that log(inf) returns
NaN, which is inconsistent with CORE​::log, which returns inf, as does other
mathematical software I have tried. So that's also a bug I need to fix.

@p5pRT
Copy link
Author

p5pRT commented Sep 17, 2015

From @ap

* Peter John Acklam <pjacklam@​gmail.com> [2015-09-17 14​:30]​:

One option is to modify objectify() so undefs pass through unmodified,
but I'm not sure if this will have other unfortunate consequences. The
tests pass with this modification, but I know from experience that the
test suite doesn't test everything.

This is also a breaking change to the API​: any downstream users of the
objectify method may have to be patched. It may be undocumented and it
may be marked internal, but interfaces that stick around forever without
changing tend to be found and used by whoever happens to need the exact
functionality they provide any time it’s not available more officially.

And there is no benefit in “punishing” such “wrongdoers” – esp. if the
offending code is in a CPAN module, the worst of the punishment often
befalls users further downstream of the “perp”, who did not do anything
“wrong” other than not audit every line of code in every module they’ve
used for breaches of interface contract like this. (And they in turn may
have yet more users who are then doubly removed from any wrongdoing but
get to pay for it anyway.)

Sometimes you have to break a few eggs to make an omelette, but that
should never happen for mere expedience… even when there is real reason
to break stuff. If too much breakage would result, sometimes that means
we just have to live with the mistakes of the past. (Again, the biggest
costs often befall users who did nothing to “deserve” them even just
a tiny little bit.)

C.f. http​://neilb.org/tag/cpan-river/

One alternative is to modify blog() to handle this special case where
the base is undef. The latter is probably the best. I can't think of
any other operator/method where undef has a special meaning.

I am opining without any real knowledge of the codebase so I don’t know
what I’m talking about, but, from just your description of the structure
of the code, that sounds like the right spot to place the special case
anyhow.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Sep 18, 2015

From @pjacklam

Please try Math-BigInt-1.999702.

The new release also fixes the case log(inf), which now returns inf, not
NaN. I noticed that there are still issues with blog() when the base is not
Euler's number, but those issues do not affect the log() function.

Peter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants