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

Test and document floating point determinsm #312

Open
jaekwon opened this issue Aug 4, 2022 · 4 comments · May be fixed by #3185
Open

Test and document floating point determinsm #312

jaekwon opened this issue Aug 4, 2022 · 4 comments · May be fixed by #3185
Assignees
Labels
🗺️good first issue🗺️ Ideal for newcomer contributors help wanted Want to contribute? We recommend these issues.

Comments

@jaekwon
Copy link
Contributor

jaekwon commented Aug 4, 2022

#306 introduced floating point and decimals, but it isn't clear if it actually is deterministic across go versions and architectures. This Issue should remain open until genesis, to ensure that the genesis validators can stay in sync.

@ajnavarro
Copy link
Contributor

I Did some research, and Go is using the IEEE 754 standard for floats:

Also, I tried with different CPU architectures and go versions checking the binary representation, which was always the same.

Protobuf, and by extension amino, are encoding floats using little-endian.

@thehowl
Copy link
Member

thehowl commented Nov 13, 2023

To provide some context on this issue, see this article.

We solve a lot of headaches by not allowing float multiply-accumulate within Gno already. That seems to be a great cause of pain in floating point determinism, particularly because x86 seems to have 80-bit precision1 for intermediate results.

In order to make this issue actionable, I propose the following tasks:

  • As this article is saying, make sure we are always using SSE for x86 floating point operations within the GnoVM. We should disable compiling without SSE/2 on x86.
  • Create a package gnovm/pkg/gnolang/floats, which contains source code for the main algebraic operators (Add, Multiply, Subtract, Divide) and implements them in assembly, trying to copy over from Go code, or Go assembly code in general.
    • Alternatively, write a test that makes sure that the generated assembly in Go for the floating point operations we use remains consistent across Go versions, so we can be alerted when this changes in a version upgrade.
    • In any case, we should make this clear in documentation for writing native injections (ie. accept floats if you have to, and make sure to remain consistent with what we do).
  • Run an assertion at startup of gno.land that ensures that floating point operations on the given machine result in a known set of values, using known "buggy" values for the four operators. If it doesn't match the known values, gno.land panics.

Footnotes

  1. See http://yosefk.com/blog/consistency-how-to-defeat-the-purpose-of-ieee-floating-point.html.

@thehowl
Copy link
Member

thehowl commented Nov 21, 2023

So, working on #1153, go 1.21 modifies this function and adds this interesting comment:

func isOddInt(x float64) bool {
	if Abs(x) >= (1 << 53) {
		// 1 << 53 is the largest exact integer in the float64 format.
		// Any number outside this range will be truncated before the decimal point and therefore will always be
		// an even integer.
		// Without this check and if x overflows int64 the int64(xi) conversion below may produce incorrect results
		// on some architectures (and does so on arm64). See issue #57465.
		return false
	}

	xi, xf := Modf(x)
	return xf == 0 && int64(xi)&1 == 1
}

I'm slowly starting to think that if we want to have floating points in Gno, they should have a software implementation which uses no hardware floating points under the hood...

@thehowl
Copy link
Member

thehowl commented Sep 25, 2024

We should bring over this file, and test/document any differences from x86 results: https://go.dev/src/runtime/softfloat64.go .

Update: #2863

@Kouteki Kouteki added help wanted Want to contribute? We recommend these issues. and removed help wanted labels Oct 2, 2024
@Kouteki Kouteki moved this from Teritori (unconfirmed) to Teritori (confirmed) in 🍜 Seoul triage Nov 19, 2024
@zxxma zxxma assigned omarsy and unassigned zxxma Nov 19, 2024
@omarsy omarsy linked a pull request Dec 1, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗺️good first issue🗺️ Ideal for newcomer contributors help wanted Want to contribute? We recommend these issues.
Projects
Status: Teritori (confirmed)
Status: Todo
Status: 🚀 Needed for Launch
7 participants