-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
math/big: add Int.Float64 conversion (was initially: {ToInt64,ToUint64,Float64}) #56984
Comments
Change https://go.dev/cl/453115 mentions this issue: |
Per https://pkg.go.dev/math/big#Accuracy:
It doesn't seem like too much of a stretch to apply that to a I'm not so sure about I guess you could argue that |
Floating point values are just as exact as Integers, they're just a different subset of the rationals. I don't see any fundamental difference in the idea that the range of Int and Float are strictly larger than int and float, and thus approximation is inevitable in conversion. Indeed, the ratio of cardinalities is much bigger (infinite) in the Int:int case. I'd be happy to adjust the wording on Accuracy to make clear that it applies to integer range conversions too. |
I guess I just don't see a compelling need for In contrast, the value of the Moreover, it is much harder to check the |
The fundamental difference is that the bound on the rounding error for a converted |
I don't think memory usage is the motivating factor here, though it is certainly true that the naive implementation of Float64() is allocation-heavy and a low-allocation version is non-trivial, which is a reason to include this operation in the API. But you're right that callers can implement efficient versions of the two {u,}int64 methods themselves without excessive difficulty, so it's fair to describe them convenience methods.
That doesn't seem very important. If the rounded Int.Float64 result is (Infinity, Above), all you really know is that the integer was too large. So too with the (MaxInt64, Above) result from Int64 and Uint64: the value was too large. In that sense these Above/Below cases are more similar to indications of infinity than minor rounding errors. Without loss of generality or efficiency, we could define the Int64 and Uint64 methods so that they return a bool instead of an Accuracy. True would mean "exact", false would mean "truncated to min[T] or max[T]". Would you prefer that? |
For But, assuming for the sake of argument that saturating really is more useful, would it make more sense to define the existing b := new(big.Int)
…
i := b.Int64()
if (i == math.MaxInt64 || i == math.MinInt64) && !b.IsInt64() {
// Conversion was inexact.
…
} That's not much (if any!) more expensive than checking the |
The existing methods have no way to report inexactness, which is the most important motivation for the new {u,}int64 methods. |
Right, but |
Fair enough. Let's retract the integer methods from the proposal then. |
I think @bcmills makes some good points. I am also in favor of simply adding func (x *Int) Float64() (float64, Accuracy) |
Anyone else object to the reduced proposal? |
The proposed implementation in https://go.dev/cl/453115 is ready for review. |
This proposal has been added to the active column of the proposals project |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
This operation converts a big.Int to float64, reporting the accuracy of the result, with a fast path in hardware. Fixes golang#56984 Change-Id: I86d0fb0e105a06a4009986f2f5fd87a4d446f6f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/453115 Reviewed-by: Robert Griesemer <gri@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Alan Donovan <adonovan@google.com>
Change https://go.dev/cl/500116 mentions this issue: |
The "To" prefix was a relic of the first draft that I failed to make consistent with the unprefixed name used in the proposal. Fortunately iant spotted it during the API audit. Updates #56984 Updates #60560 Change-Id: Ifa6eeddf6dd5f0637c0568e383f9a4bef88b10f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/500116 Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Alan Donovan <adonovan@google.com>
The
Int
type in themath/big
package represents an arbitrary-precision integer. Today, it provides methods calledInt64
andUint64
, which return the integer in theint64
anduint64
(machine) representations. However, it doesn't indicate whether the conversion is exact, so the caller must ascertain this, perhaps by a prior call toIsInt64
orIsUint64
; otherwise, the result is undefined.I propose to add three new methods to the package:
Int.{ToInt64,ToUint64,Float64}
. All three follow the same pattern of returning the closest representable value, and abig.Accuracy
enum indicating whether the conversion was exact, a rounding up, or a rounding down. This matches the API ofbig.Float
, which provides similar conversions to several types, includingInt
.The
To
prefix is needed forToInt64
andToUint64
because the unprefixed names are taken.Implementation sketch in https://go.dev/cl/453115. Tests to be added later, along with optimizations, such as a fast path of Float64 for 64-bit values.
@griesemer
The text was updated successfully, but these errors were encountered: