-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implement floating point conversion #110
base: master
Are you sure you want to change the base?
Conversation
Fixes Feature request: toBiggestInt nim-lang#34
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I'm not sure this feature is really worth implementing. Imo we shouldn't just implement every feature request without first thinking about it. Do you have any concrete use case? Is there any precedent in some standard bigint implementation out there?
src/bigints.nim
Outdated
@@ -1140,6 +1140,11 @@ func fastLog2*(a: BigInt): int = | |||
return -1 | |||
bitops.fastLog2(a.limbs[^1]) + 32*(a.limbs.high) | |||
|
|||
func toBiggestFloat*(x: BigInt): BiggestFloat = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func toBiggestFloat*(x: BigInt): BiggestFloat = | |
func toFloat*[T: SomeFloat](x: BigInt): Option[T] = |
IMO there's no reason to special case BiggestFloat
(which is float64
currently). The implementation should work the same with 32 bit floats. I also think returning an Option
makes more sense, since not every BigInt
can be represented as a float (unless we just round).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the Option (see recently closed PR for BigInt -> integer type conversion).
For the examples, you want to test the saturation values and the one above.
See : https://en.wikipedia.org/wiki/Double-precision_floating-point_format#Double-precision_examples for some values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to provide +Inf, -Inf, subnormal values instead of an Option though. I prefer the Option solution, since we lose (almost) all information by converting. The option may warn the user of the conversion problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The reason to special case BiggestFloat is because it's modeled after https://nim-lang.org/docs/system.html#toBiggestFloat%2CBiggestInt
- The API for
toFloat
is ugly since there is no way to deduce the type parameter. I'm avoiding a generic interface since there's no clear guidance on when to use generics versus typedesc parameters. [RFC] guidelines for when to use typedesc vs generics RFCs#40 BigInt
to integer is a very, very different case. Floating point math is, by nature, approximate. It is expected to be a lossy conversion, so rounding is the right thing to do here. I added some tests for the Infinite cases.
BTW, in looking into this, I also discovered this bug nim-lang/Nim#20102
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The reason to special case BiggestFloat is because it's modeled after https://nim-lang.org/docs/system.html#toBiggestFloat%2CBiggestInt
I don't see that as a good reason to repeat this here.
- The API for
toFloat
is ugly since there is no way to deduce the type parameter. I'm avoiding a generic interface since there's no clear guidance on when to use generics versus typedesc parameters. [RFC] guidelines for when to use typedesc vs generics RFCs#40
Wether that's ugly or not is quite subjective. I don't see a problem with toFloat[float64](...)
(it isn't much more to type than toBiggestFloat
). I don't see why we should special case BiggestFloat
(aka float64
).
BigInt
to integer is a very, very different case. Floating point math is, by nature, approximate. It is expected to be a lossy conversion, so rounding is the right thing to do here. I added some tests for the Infinite cases.
That's a fair point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. So why proc toFloat[T:SomeFloat](x: BigInt):T
and not proc toFloat(x: BigInt, T: type SomeFloat):T
or even proc to(x:BigInt, T:type SomeFloat):T
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because imo you should use generics for passing type parameters. I know that not everyone agrees with this though. If we ever get better type inference (which I really hope we do), the generic parameter here could even be inferred, based on subsequent uses of the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switched to func to*(x: BigInt, T:type SomeFloat): T
This has the benefit of not repeating "float" in calls (x.toFloat[float32]()
).
I didn't use generics for the following reasons:
- Generic specialization does not work. (
Error: overloaded 'to' leads to ambiguous calls
if I try to use generics and add ato[SomeInteger]
). - In method call syntax, it's ambiguous with an indexer (
x.to[:float32]()
leads to an unintuitive error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I would actually support to implement toBiggestFloat
instead of conversion for all types of float.
The reason to special case BiggestFloat is because it's modeled after https://nim- lang.org/docs/system.html#toBiggestFloat%2CBiggestInt
I don't see that as a good reason to repeat this here.
For me that is a good reason. It means having a consistent API across nim ecosystem (and indeed bigints api is modeled after system api). It would not be a good reason if toBiggestFloat
in system was originally a bad api (so we should not copy it), but I do not see why that would be the case.
I don't see why we should special case BiggestFloat (aka float64).
Because the use case call for converting to a float and it make sense to convert to biggest float you have.
I do not think we should support conversion to multiple floats, since that would mean that the user would have to think: "which float should I use? why would I want to convert to a smaller float?"
I've modeled it off of https://nim-lang.org/docs/system.html#toBiggestFloat%2CBiggestInt
|
Can you elaborate? I don't see how conversion to float is needed for that. |
Instead of reimplementing display formatting, float conversion allows you to leverage existing tools: https://nim-lang.org/docs/strformat.html#formatting-floats Also, floating point allows you to do operations that may not be an integer, such as square roots. While no floating point can represent irrational numbers, an estimate may be good enough. |
I added a note above on why I actually think |
Large BigInt values cannot be converted to float64 or they just become inf. import std/[math, strformat, options]
import bigints
proc approximate10(x: BigInt; precision: range[1..18]): tuple[mantissa, exponent: int] =
# int.high.float.log10 ≒ 18.96
let
# https://en.wikipedia.org/wiki/Logarithm#Change_of_base
log10 = (x.fastLog2.float / 3.321928).int # log2(10) = 3.321928
exp = max(0, log10 - precision)
((x div 10.initBigInt.pow(exp)).toInt[:int].get, exp)
proc test =
block:
let
a = 10.initBigInt.pow(40)
(m, e) = a.approximate10(5)
echo a
echo &"≒ {m} * 10^{e}"
block:
let
a = 1234567890.initBigInt
(m, e) = a.approximate10(3)
echo a
echo &"≒ {m} * 10^{e}"
test()
proc sqrt(x: BigInt; maxIter: int): BigInt =
# Quick approximated value
#result = 1.initBigInt shl (x.fastLog2 div 2)
#result = x shr (x.fastLog2 div 2 + 1)
let
log2 = x.fastLog2
b2 = (x shr (log2 - 1)).toInt[:int].get and 1
result = (1.initBigInt shl (log2 div 2)) or ((log2 mod 2).initBigInt shl (log2 div 2 - 2 + b2))
# Refine value with Newton's method
# https://en.wikipedia.org/wiki/Newton's_method
for i in 1..maxIter:
result = (result * result + x) div (result * 2.initBigInt)
proc testSqrt =
block:
var a = (256 + 128).initBigInt
echo a
echo a.sqrt(0)
echo a.sqrt(1)
echo a.sqrt(2)
echo a.sqrt(3)
block:
var a = 4000000.initBigInt
echo a
echo a.sqrt(0)
echo a.sqrt(1)
echo a.sqrt(2)
echo a.sqrt(3)
block:
var a = 6000000.initBigInt
echo a
echo a.sqrt(0)
echo a.sqrt(1)
echo a.sqrt(2)
echo a.sqrt(3)
block:
var a = 8000000.initBigInt
echo a
echo a.sqrt(0)
echo a.sqrt(1)
echo a.sqrt(2)
echo a.sqrt(3)
block:
var a = 9000000.initBigInt
echo a
echo a.sqrt(0)
echo a.sqrt(1)
echo a.sqrt(2)
echo a.sqrt(3)
testSqrt() Example output:
This is quick implementation and not much tested. |
I think you were trying to demonstrate this PR is unnecessary, but have shown that the opposite. Note that Also, your Newton iteration converges on the wrong result due to quantization error. |
Yes. There is. In GMP : https://gmplib.org/manual/Converting-Integers
Sorry, that is not a bigint implementation of the standard library of a language. |
I just used When you need an accurate squre root of I'm not sure this feature is really needed or not. Highest exponent of float32/64 is 2^127 / 2^1023. I think this PR is necessary if there are use cases like, you need a large int more than 256bits but final result is smaller than 2^1023, and you need to write it in specific format that have only float64 or pass it to libraries or API that takes only float64. |
@demotomohiro Optimization is premature here, but conversion to a float is a CONSTANT time operation --- it's faster than a bigint addition, and certainly faster than your Newton iteration. @dlesnoff |
@rotu |
Do you really want to write and check this sort of code for every non-arithmetic function you might use? Furthermore, this implementation betrays its inaccuracy. You have slight accuracy issues when the square root requires looking at more digits than are in the significand. And returning the result as a bigint falsely implies the result is precise down to the unit. The correct approach for an "exact" square root of a bigint At any rate, while I personally have written code that requires values greater than 2^64, I've never had to use values larger than 2^1023! My PR is only geared toward the approach where you need an approximate answer - that's why I make no attempt to introduce a floating-point to bigint conversion. |
IEEE_754 64bit float has 53bit significand bits.
GMP has
If you don't use values larger than 2^1023, fixed size int types like https://github.com/status-im/nim-stint might be better than bigints. |
Yes. This is NOT necessarily correct down to the unit. Returning the value as a
I love when I have the same idea as smarter people than myself!
True. But this is the 'bigints' repo. I'll defer to the judgement of package maintainers whether this functionality is appropriate here. |
Fixes Feature request: toBiggestInt #34