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

math: add exported variables +Inf, -Inf, +0, -0 #17063

Closed
xiofen opened this issue Sep 11, 2016 · 22 comments
Closed

math: add exported variables +Inf, -Inf, +0, -0 #17063

xiofen opened this issue Sep 11, 2016 · 22 comments

Comments

@xiofen
Copy link

xiofen commented Sep 11, 2016

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 .

@josharian josharian changed the title Feature (minor) - math package - add simple support to generate +Inf, -Inf, +0, -0 math: add exported variables +Inf, -Inf, +0, -0 Sep 11, 2016
@n10v
Copy link
Contributor

n10v commented Sep 11, 2016

Made a change for it: https://go-review.googlesource.com/#/c/29019/

@cznic
Copy link
Contributor

cznic commented Sep 11, 2016

This is not a bug but a feature request. The CL is premature w/o a decision on the proposal.

@n10v
Copy link
Contributor

n10v commented Sep 11, 2016

Of course
I made this change just for a discussion

@gopherbot
Copy link

CL https://golang.org/cl/29019 mentions this issue.

@xiofen
Copy link
Author

xiofen commented Sep 11, 2016

Also should included NaN in the same format for consistency if suggestion implemented.

@n10v
Copy link
Contributor

n10v commented Sep 11, 2016

But there is already a NaN in math package and it's a function, so one more NaN can't be added.

@xiofen
Copy link
Author

xiofen commented Sep 11, 2016

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"

@xiofen
Copy link
Author

xiofen commented Sep 11, 2016

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.

@martisch
Copy link
Contributor

martisch commented Sep 11, 2016

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.

@n10v
Copy link
Contributor

n10v commented Sep 11, 2016

@martisch this solution is non-compatible with older versions of Go

@martisch
Copy link
Contributor

martisch commented Sep 11, 2016

@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.

@bradfitz
Copy link
Contributor

Instead of adding new names we could intrinsify math.Nan() and others in the compiler backend like the atomics.

It's already inlined, no?

package main

import "math"  

var v float64

func main() {
        v = math.Inf(1)
}
$ go tool compile -m -m ~/inf.go
/home/bradfitz/inf.go:7: can inline main as: func() { v = math.Inf(1) }
/home/bradfitz/inf.go:8: inlining call to math.Inf func(int) float64 { var math.v·3 uint64; math.v·3 = <N>; if math.sign·2 >= int(0) { math.v·3 = uint64(9218868437227405312) } else { math.v·3 = uint64(18442240474082181120) }; return math.Float64frombits(math.v·3) }
/home/bradfitz/inf.go:8: inlining call to math.Float64frombits func(uint64) float64 { return *(*float64)(unsafe.Pointer(&math.b·2)) }
/home/bradfitz/inf.go:8: main &math.b·2 does not escape

$ go tool compile -S ~/inf.go  
"".main t=1 size=51 args=0x0 locals=0x10
        0x0000 00000 (/home/bradfitz/inf.go:7)  TEXT    "".main(SB), $16-0
        0x0000 00000 (/home/bradfitz/inf.go:7)  SUBQ    $16, SP  
        0x0004 00004 (/home/bradfitz/inf.go:7)  MOVQ    BP, 8(SP)  
        0x0009 00009 (/home/bradfitz/inf.go:7)  LEAQ    8(SP), BP
        0x000e 00014 (/home/bradfitz/inf.go:7)  FUNCDATA        $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)  
        0x000e 00014 (/home/bradfitz/inf.go:7)  FUNCDATA        $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
        0x000e 00014 (/home/bradfitz/inf.go:8)  MOVQ    $9218868437227405312, AX
        0x0018 00024 (/home/bradfitz/inf.go:8)  MOVQ    AX, math.b·2(SP)
        0x001c 00028 (/home/bradfitz/inf.go:8)  MOVSD   math.b·2(SP), X0
        0x0021 00033 (/home/bradfitz/inf.go:8)  MOVSD   X0, "".v(SB)
        0x0029 00041 (/home/bradfitz/inf.go:9)  MOVQ    8(SP), BP
        0x002e 00046 (/home/bradfitz/inf.go:9)  ADDQ    $16, SP
        0x0032 00050 (/home/bradfitz/inf.go:9)  RET
        0x0000 48 83 ec 10 48 89 6c 24 08 48 8d 6c 24 08 48 b8  H...H.l$.H.l$.H.
        0x0010 00 00 00 00 00 00 f0 7f 48 89 04 24 f2 0f 10 04  ........H..$....
        0x0020 24 f2 0f 11 05 00 00 00 00 48 8b 6c 24 08 48 83  $........H.l$.H.
        0x0030 c4 10 c3                                         ...
        rel 37+4 t=14 "".v+0

Likewise with NaN.

I don't think there's anything to do here.

@ianlancetaylor
Copy link
Contributor

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.

@martisch
Copy link
Contributor

martisch commented Sep 11, 2016

@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):

    0x000e 00014 (main.go:14)   MOVQ    $9221120237041090561, AX
    0x0018 00024 (main.go:11)   MOVQ    AX, math.b·2+24(SP)
    0x001d 00029 (main.go:11)   MOVSD   math.b·2+24(SP), X0
    0x0023 00035 (main.go:11)   MOVSD   X0, "".f(SB)
    0x002b 00043 (main.go:12)   MOVQ    AX, math.b·2+16(SP)
    0x0030 00048 (main.go:12)   MOVSD   math.b·2+16(SP), X0
    0x0036 00054 (main.go:12)   MOVSD   X0, "".f(SB)
    0x003e 00062 (main.go:13)   MOVQ    AX, math.b·2+8(SP)
    0x0043 00067 (main.go:13)   MOVSD   math.b·2+8(SP), X0
    0x0049 00073 (main.go:13)   MOVSD   X0, "".f(SB)
    0x0051 00081 (main.go:14)   MOVQ    AX, math.b·2(SP)
    0x0055 00085 (main.go:14)   MOVSD   math.b·2(SP), X0
    0x005a 00090 (main.go:14)   MOVSD   X0, "".f(SB)
    0x0062 00098 (main.go:15)   MOVQ    32(SP), BP
    0x0067 00103 (main.go:15)   ADDQ    $40, SP
    0x006b 00107 (main.go:15)   RET

intrinsified (local CL):

    0x0000 00000 (main.go:10)   TEXT    "".test(SB), $0-0
    0x0000 00000 (main.go:10)   FUNCDATA    $0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    0x0000 00000 (main.go:10)   FUNCDATA    $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
    0x0000 00000 (main.go:11)   MOVSD   $f64.7ff8000000000001(SB), X0
    0x0008 00008 (main.go:14)   MOVSD   X0, "".f(SB)
    0x0010 00016 (main.go:15)   RET

@griesemer
Copy link
Contributor

griesemer commented Sep 12, 2016

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: x := 0.0; x = -x and now x is -0.

I vote against this proposal/suggestion.

@griesemer
Copy link
Contributor

@xiofen As an aside, your example code

if a/b == math.MinusZero {..}

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:

if math.Copysign(1, a/b) < 0 ...

That is, the readability argument you brought up would actually lead to incorrect code. Another reason against this proposal.

@bradfitz
Copy link
Contributor

Closing. Many arguments against.

@xiofen
Copy link
Author

xiofen commented Sep 13, 2016

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

float32(1)/float32(0)

returns a compiler error - which is part of the reason why this request was filed.

Perhaps that could be fixed instead ?

@griesemer
Copy link
Contributor

@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 x := 0.0; x = 1/x and now x is +Inf ( https://play.golang.org/p/sKPmyVUAd0 ).

@xiofen
Copy link
Author

xiofen commented Sep 13, 2016

lol I have to define a global zerovar zero float32 = 0 to get access to builtin float32 Inf without importing the math package ...

:| (thousand yard stare)

ok

@bradfitz
Copy link
Contributor

@xiofen, it doesn't need to be global. Also, you can just import "math" and then write:

float32(math.Inf(1))

@griesemer
Copy link
Contributor

@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.

@golang golang locked and limited conversation to collaborators Sep 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants