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: add exported variables +Inf, -Inf, +0, -0 #17063
Comments
Made a change for it: https://go-review.googlesource.com/#/c/29019/ |
This is not a bug but a feature request. The CL is premature w/o a decision on the proposal. |
Of course |
CL https://golang.org/cl/29019 mentions this issue. |
Also should included NaN in the same format for consistency if suggestion implemented. |
But there is already a NaN in math package and it's a function, so one more NaN can't be added. |
maybe the others should be functions eg. math.Infinity() rather than exported variables - this was noted in the goland-nuts discussion - the function cannot have its value altered - unlike the exported variable - which makes sense in this context - the values should be "protected" |
Whilst under discussion.. float32 and float64 are builtin types, defined in core spec (https://golang.org/ref/spec#Numeric_types), with a usage that extends beyond the math package. Inf, NaN, -0 are all generatable from builtin operations eg / (divide) As IEEE 754 (single and double) is supported by core spec, so, there's an argument that NaN, Inf, -0 should also be supported by the core spec - ie should be in builtin (https://golang.org/pkg/builtin) ok that's a big ask, with a world of naming issues for code already written.. Nevertheless I think it makes some good sense - so I should bring it up. |
Instead of adding new names we could intrinsify math.Nan() and others in the compiler backend like the atomics. It seems to me that this way no code has to be changed or can be written in more ways than before, its as readable as before and as fast as having constants. |
@martisch this solution is non-compatible with older versions of Go |
@BoGeM could you clarify what you mean by non-compatible (i assume you are talking about my suggestion)? I do not see any visible language change when lowering the function calls to values in the compiler but maybe i am missing a detail. |
It's already inlined, no? package main
import "math"
var v float64
func main() {
v = math.Inf(1)
}
Likewise with I don't think there's anything to do here. |
Is there an argument in favor of this beyond "it seems useful?" Because there are many things that are useful. If we add these to the math package, then next year someone will come along and ask why the math package has multiple ways of doing exactly the same thing. |
@bradfitz yes its inlined which is good but plain substitution with a constant by the compiler might be even better. But it is a minor point. I need to have a closer look but inlining seems to leave some overhead around which might make a difference when avoided. But nothing that needs to keep the specific issue open. first try at it: 4x math.Nan() assigned to a global: inlined (currently):
intrinsified (local CL):
|
What @ianlancetaylor said: Unless there's a compelling (as in "extremely strong") argument for this, I don't see any reason to make the API bigger. First of all, infinities may be meaningful results as part of a computation (per IEEE floating-point behavior), but they are rarely input and thus we rarely need the actual values. And we can create an infinity easily enough. If there's a performance issue, in creating them, one can trivially create one and store it locally. The case for NaNs is even more rare. Furthermore, there's many NaNs. Anybody who has to do more with NaNs but testing for them, will probably want to have full control over the bits (which they already have). Testing for a NaN is trivial: x != x for NaNs. This leaves -0; and in fact -0 might be the only value where it might be useful to have an easier way to access one besides with Copysign. math/big needs a few -0's in it's code, mostly for IEEE compatibility when converting to float32/64. Note that we cannot have a -0 constant, because Go constants don't support negative zeros. Thus, we could have a math.MinusZero variable. But that runs the risk that somebody modifies that value; so we would need a MinusZero() function. At which point we can just as well call Copysign. But that leaves away the elementary solution which doesn't even require package math which is: I vote against this proposal/suggestion. |
@xiofen As an aside, your example code
is not correct anyway: Comparing a -0 against 0 is true as well per IEEE, so your code won't work in this form whether you have a -0 value readily available or not. This is exactly the reason why you need Copysign. The correct code to test for a -0 result is:
That is, the readability argument you brought up would actually lead to incorrect code. Another reason against this proposal. |
Closing. Many arguments against. |
Quote : " But that leaves away the elementary solution which doesn't even require package math which is: x := 0.0; x = -x and now x is -0." Yes that is simple - and generating +Inf etc would be simple too by similar methods - except
returns a compiler error - which is part of the reason why this request was filed. Perhaps that could be fixed instead ? |
@xiofen No, that does not need to be fixed; it's a constant operation and constants don't have infinities. Again, why don't you use the elementary operation |
lol I have to define a global zero :| (thousand yard stare) ok |
@xiofen, it doesn't need to be global. Also, you can just
|
@xiofen Please use the public forums for further discussion. We don't use the issue tracker to discuss programming style; also this issue is closed. Thanks. |
Minor request - support assignment of IEEE 754 signed zeros, and non-numbers in the package in the simplest possible form
I brought this up here https://groups.google.com/forum/#!topic/golang-nuts/BVyo-QQO8xg
The question was answered, and there are ways to generate all these
+Inf = math.Inf(+1)
-Inf = math.Inf(-1)
-0 = math.Copysign(0, -1)
NaN = math.NaN()
It seemed a useful thing to have these defined as exportable variables at the beginning of the package eg math.Infinity, math.MinusInfinity, math.NaN, math.MinusZero
Rationale - ease of use, simplicity, makes for clearer code .. eg 👍
if a/b == math.MinusZero {..}
rather than
if a/b == math.CopySign(0,-1) {..}
I see that math.NaN() is already supported as a function not variable - so maybe the rest should follow this pattern..
Thanks .
The text was updated successfully, but these errors were encountered: