Fix: calculate `log10` for a bigint with value zero (fixes #3539)
Addresses the first case of log10(0n) in #3539.
We may want to think through the behavior for all numeric types. Current behavior (with this bugfix applied):
| data type | test | predictable:false |
predictable:true |
|---|---|---|---|
| number | math.log10(0) |
-Infinity |
-Infinity |
| BigNumber | math.log10(math.bignumber('0')) |
bignumber('-Infinity') |
bignumber('-Infinity') |
| bigint | math.log10(0n) |
complex(Infinity, Infinity) |
NaN |
| boolean | math.log10(false) |
-Infinity |
-Infinity |
For consistency, it may be better to return complex(Infinity, Infinity) for every data type. Or the other way around:
return -Infinity in case of a bigint (depending on the configured numberFallback).
Any thoughs on this @gwhitney?
For consistency, it may be better to return
complex(Infinity, Infinity)for every data type. Or the other way around: return-Infinityin case of a bigint (depending on the configurednumberFallback).
Yes I think there's no reason to go to Complex for the particular case of 0n. Since it returns number for positive bigint, the number -Infinity would make more sense. I guess that needs to be special-cased in promoteLogarithm...
Ok we can change that. And of course for negative values we still evaluate to a complex result.
With the latest update in this PR, the behavior is now as follows, and I think that's OK:
| data type | test | predictable:false |
predictable:true |
|---|---|---|---|
| number | math.log10(0) |
-Infinity |
-Infinity |
| BigNumber | math.log10(math.bignumber('0')) |
bignumber('-Infinity') |
bignumber('-Infinity') |
| bigint | math.log10(0n) |
-Infinity |
NaN |
| boolean | math.log10(false) |
-Infinity |
-Infinity |
Oops, I have now lost the thread of why when predictable is true, the log10(0n) should switch from -Infinity to NaN? "Predictable: true" means that the type of the output should be determined by the type of the input, with no dependence on the value of the input. The "predictable" signature for log10 on a bigint is to return an (ordinary JavaScript) number, and -Infinity is a perfectly good number, so why wouldn't we just return it even when predictable is true? It is definitely a better representation of log10(0n) than NaN is.
What does seem correct is that for negative arguments, when predictable is true, log10 of a bigint should return NaN, since there is no number that will serve a the log10 of a negative value. (Note it is quite possible I wrote that code, and if so, mea culpa, my apologies... but as long as we are working on it, we should correct it, if you agree with my analysis...