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: discrepancy in Atan2 special case #35446

Open
bmkessler opened this issue Nov 8, 2019 · 4 comments
Labels
Milestone

Comments

@bmkessler
Copy link
Contributor

@bmkessler bmkessler commented Nov 8, 2019

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

yes

The special cases for Atan2 are

// Special cases are (in order):
//	Atan2(y, NaN) = NaN
//	Atan2(NaN, x) = NaN
//	Atan2(+0, x>=0) = +0
//	Atan2(-0, x>=0) = -0
//	Atan2(+0, x<=-0) = +Pi
//	Atan2(-0, x<=-0) = -Pi
//	Atan2(y>0, 0) = +Pi/2
//	Atan2(y<0, 0) = -Pi/2
//	Atan2(+Inf, +Inf) = +Pi/4
//	Atan2(-Inf, +Inf) = -Pi/4
//	Atan2(+Inf, -Inf) = 3Pi/4
//	Atan2(-Inf, -Inf) = -3Pi/4
//	Atan2(y, +Inf) = 0
//	Atan2(y>0, -Inf) = +Pi
//	Atan2(y<0, -Inf) = -Pi
//	Atan2(+Inf, x) = +Pi/2
//	Atan2(-Inf, x) = -Pi/2

Atan2(y, +Inf) = 0 is handled differently between the pure go code and the s390x implementation.

The pure go implementation actually conforms to C99 Annex F. 9.1.4 which specifies:

atan2(±y,+∞) returns ± 0 for finite y>0.

Specifically, the sign of zero depends on the sign of y, while the s390x implementation returns +0 for all values of y.

Note that this behavior affects special case handling of math/cmplx as well since those functions rely on math for the underlying implementations.

A decision should be made on what the specification should state for the sign of zero in this case. I am not sure if updating the go specification to depend on the sign of y (C99) would be incompatible with the go compatibility promise since the current specification doesn't explicitly state that the sign should be positive. It seems that the majority implementation is already conforming to the C99 specification in any case.

@bmkessler

This comment has been minimized.

Copy link
Contributor Author

@bmkessler bmkessler commented Nov 8, 2019

@bmkessler

This comment has been minimized.

Copy link
Contributor Author

@bmkessler bmkessler commented Nov 8, 2019

This causes s390x test failures on #29320 for cmplx.Log and cmplx.Log10

@bcmills bcmills added this to the Backlog milestone Nov 8, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Nov 8, 2019

I am not sure if updating the go specification to depend on the sign of y (C99) would be incompatible with the go compatibility promise since the current specification doesn't explicitly state that the sign should be positive.

In general the math functions aim for consistency with their C equivalents, I believe. And certainly the Go compatibility policy allows the implementation to be made consistent across platforms.

@mundaym

This comment has been minimized.

Copy link
Member

@mundaym mundaym commented Nov 8, 2019

I'm happy to add the Atan2(±y, +Inf) = ±0 special case to the s390x implementation if that helps. I don't think we necessarily need to update the documentation for Atan2 but it would be good to add a test to enforce this special case even if we don't guarantee it externally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.