golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.42k stars 17.4k forks source link

math: Different results using x86 and x64 versions of Go 1 #3423

Closed gopherbot closed 9 years ago

gopherbot commented 12 years ago

by phrozen10:

Hi, today I installed Go 1 windows 386 on my office machine and run a program I've been
working on to learn Go. Everything went fine and the output image rendered correctly.
Then I came home and installed Go 1 windows amd64 version, just to find out the
resulting image was totally awkward, I uninstalled the amd64 version and installed 386
and run the program and the output went as expected.

Here is the program
https://gist.github.com/2231611 
go run main.go <-- should produce a png, no other than std libs required, vanilla
installation of Go 1

http://tinypic.com/r/2ec29g1/5 <-- 386 (correct)
http://tinypic.com/r/34o5xu1/5 <-- amd64 (wrong)

I use float64 on most of the calculations apart from uint8 for the colors as expected to
the 'image/color' package. Some int here and there mainly for index.

*Attached the files to the issue.

Attachments:

  1. main.go (4922 bytes)
  2. go1_windows_386.png (1274572 bytes)
  3. go1_windows_amd64.png (1661377 bytes)
nigeltao commented 12 years ago

Comment 1:

Owner changed to @nigeltao.

nigeltao commented 12 years ago

Comment 2:

The fundamental difference is demonstrated by this trivial program:
func main() {
  x := 257.0
  println(uint8(x))
}
6g: prints 1
8g: prints 0
iant tells me that gccgo prints 1 in 64-bit mode and 255 in 32-bit mode. I'll re-assign
the bug to him, and let iant/rsc decide where to take it.
phrozen10, a workaround is to change the lines in func HSVToRGB from
    r := uint8((fR * 255) + 0.5)
    g := uint8((fG * 255) + 0.5)
    b := uint8((fB * 255) + 0.5)
to
    conv := func(x float64) uint8 {
        if x < 0 {
            return 0
        }
        if x > 1 {
            return 255
        }
        return uint8(int(x*255 + 0.5))
    }
    r, g, b := conv(fR), conv(fG), conv(fB)
That should give you similar colors on both 32-bit and 64-bit.
Even so, the actual image produced will differ between the 32-bit and 64-bit versions.
The sin/cos implementations are slightly different on each (IIUC 32-bit uses 80-bit
floats for intermediate values, 64-bit uses 64-bit floats throughout), and even though
the differences are initially small, chaotic attractors will magnify any small
perturbation.

Owner changed to @ianlancetaylor.

remyoudompheng commented 12 years ago

Comment 3:

There seems to be a dedicated test in test/intcvt.go but it's commented...
gopherbot commented 12 years ago

Comment 4 by phrozen10:

Thank you Nigel. I'll make the necessary modifications you stated and test over 64-bit.
About the sin and cos implementations, I'll check if there are any differences, though I
add random noise to the attractor coordinates from (-0.0025, 0.0025) on each iteration,
so I don't think the small deviation would differ a lot from that. I may have to take
out the blur to check for differences. They are chaotic after all.
I'll keep checking here for any updates on the issue. Thank you again for your insight.
remyoudompheng commented 12 years ago

Comment 5:

In the spec: "In all non-constant conversions involving floating-point or complex
values, if the result type cannot represent the value the conversion succeeds but the
result value is implementation-dependent."
minux commented 12 years ago

Comment 6:

FYI, on 386, we disable extended precision mode for FPU, so we will
always use 64-bit floating point on all supported architectures.
nigeltao commented 12 years ago

Comment 7:

Ah, I might have been too hasty to blame 80-bit vs 64-bit. Still, in
$GOROOT/src/pkg/math, it looks like sincos_386.s and sincos_amd64.s use different
algorithms.
cldorian commented 12 years ago

Comment 8:

Re comment 7: The 386 code uses the x87 math coprocessor, which, in this case, has a
relative error of 4e-16. The amd64 code uses an algorithm that is even more accurate.
dsymonds commented 12 years ago

Comment 9:

Labels changed: added priority-later, removed priority-triage.

Status changed to Accepted.

rsc commented 12 years ago

Comment 10:

I don't believe there's anything to fix here, sadly.
In all non-constant conversions involving floating-point or complex values, if the
result type cannot represent the value the conversion succeeds but the result value is
implementation-dependent.
 - http://golang.org/ref/spec#Conversions
You can't convert 257.0 to a byte and expect a specific answer.

Status changed to Unfortunate.

gopherbot commented 12 years ago

Comment 11 by phrozen10:

In all non-constant conversions involving floating-point or complex values, if the
result type cannot represent the value the conversion succeeds but the result value is
implementation-dependent.
 - http://golang.org/ref/spec#Conversions
At least make sure that official go implementations handle it the same way? :S
rsc commented 12 years ago

Comment 12:

Implementation-dependent is implementation-dependent.
It's not required for all implementations to behave the same way.
gopherbot commented 12 years ago

Comment 13 by phrozen10:

I can read the spec and understand it is NOT required.
Although In my opinion it would be a quality feature that at least gc implementations
behave the same way in different architectures following the POLA. Other implementations
like gccgo can behave as they like.
rsc commented 12 years ago

Comment 14:

Usually the reason a spec says something is implementation-dependent
is because the easiest/most natural/most efficient choices depends on
details of the surrounding implementation.  If we didn't want that
flexibility and were willing to make the implementations all agree, we
wouldn't say it was implementation-dependent.
ianlancetaylor commented 11 years ago

Comment 15:

Issue #3966 has been merged into this issue.