Skip to content

Commit

Permalink
logictest: compare floating point values approximately on s390x
Browse files Browse the repository at this point in the history
On s390x in the std math package and some c-deps, floating point
calculations can produce results that differ from the values calculated
on amd64. This patch adds a function to compare logictest floating point
and decimal values within a small relative margin on s390x.

Release note: None
  • Loading branch information
jonathan-albrecht-ibm committed Apr 8, 2021
1 parent aa4d1f0 commit b3237ed
Show file tree
Hide file tree
Showing 7 changed files with 302 additions and 10 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ ALL_TESTS = [
"//pkg/storage/enginepb:enginepb_test",
"//pkg/storage/metamorphic:metamorphic_test",
"//pkg/storage:storage_test",
"//pkg/testutils/floatcmp:floatcmp_test",
"//pkg/testutils/keysutils:keysutils_test",
"//pkg/testutils/lint/passes/fmtsafe:fmtsafe_test",
"//pkg/testutils/lint/passes/forbiddenmethod:forbiddenmethod_test",
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ go_library(
"//pkg/sql/sessiondata",
"//pkg/sql/stats",
"//pkg/testutils",
"//pkg/testutils/floatcmp",
"//pkg/testutils/physicalplanutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
Expand Down
45 changes: 37 additions & 8 deletions pkg/sql/logictest/logic.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"path/filepath"
"reflect"
"regexp"
"runtime"
"runtime/debug"
"runtime/trace"
"sort"
Expand Down Expand Up @@ -57,6 +58,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/stats"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/floatcmp"
"github.com/cockroachdb/cockroach/pkg/testutils/physicalplanutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
Expand Down Expand Up @@ -2832,9 +2834,16 @@ func (t *logicTest) execQuery(query logicQuery) error {
// To find the coltype for the given result, mod the result number
// by the number of coltypes.
colT := query.colTypes[i%len(query.colTypes)]
if !resultMatches && colT == 'F' {
if !resultMatches {
var err error
resultMatches, err = floatsMatch(expected, actual)
// On s390x, check that values for both float ('F') and decimal
// ('R') coltypes are approximately equal to take into account
// platform differences in floating point calculations.
if runtime.GOARCH == "s390x" && (colT == 'F' || colT == 'R') {
resultMatches, err = floatsMatchApprox(expected, actual)
} else if colT == 'F' {
resultMatches, err = floatsMatch(expected, actual)
}
if err != nil {
return errors.CombineErrors(makeError(), err)
}
Expand Down Expand Up @@ -2892,17 +2901,37 @@ func (t *logicTest) execQuery(query logicQuery) error {
return nil
}

// floatsMatch returns whether two floating point numbers represented as
// strings have matching 15 significant decimal digits (this is the precision
// that Postgres supports for 'double precision' type).
func floatsMatch(expectedString, actualString string) (bool, error) {
// parseExpectedAndActualFloats converts the strings expectedString and
// actualString to float64 values.
func parseExpectedAndActualFloats(expectedString, actualString string) (float64, float64, error) {
expected, err := strconv.ParseFloat(expectedString, 64 /* bitSize */)
if err != nil {
return false, errors.Wrap(err, "when parsing expected")
return 0, 0, errors.Wrap(err, "when parsing expected")
}
actual, err := strconv.ParseFloat(actualString, 64 /* bitSize */)
if err != nil {
return false, errors.Wrap(err, "when parsing actual")
return 0, 0, errors.Wrap(err, "when parsing actual")
}
return expected, actual, nil
}

// floatsMatchApprox returns whether two floating point represented as
// strings are equal within a tolerance.
func floatsMatchApprox(expectedString, actualString string) (bool, error) {
expected, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
if err != nil {
return false, err
}
return floatcmp.EqualApprox(expected, actual, floatcmp.CloseFraction, floatcmp.CloseMargin), nil
}

// floatsMatch returns whether two floating point numbers represented as
// strings have matching 15 significant decimal digits (this is the precision
// that Postgres supports for 'double precision' type).
func floatsMatch(expectedString, actualString string) (bool, error) {
expected, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
if err != nil {
return false, err
}
// Check special values - NaN, +Inf, -Inf, 0.
if math.IsNaN(expected) || math.IsNaN(actual) {
Expand Down
13 changes: 11 additions & 2 deletions pkg/sql/logictest/logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ func TestSqlLiteLogic(t *testing.T) {
RunSQLLiteLogicTest(t, "" /* configOverride */)
}

// TestFloatsMatch is a unit test for floatsMatch() function.
// TestFloatsMatch is a unit test for floatsMatch() and floatsMatchApprox()
// functions.
func TestFloatsMatch(t *testing.T) {
defer leaktest.AfterTest(t)()
for _, tc := range []struct {
Expand All @@ -60,7 +61,15 @@ func TestFloatsMatch(t *testing.T) {
t.Fatal(err)
}
if match != tc.match {
t.Fatalf("wrong result on %v", tc)
t.Fatalf("floatsMatch: wrong result on %v", tc)
}

match, err = floatsMatchApprox(tc.f1, tc.f2)
if err != nil {
t.Fatal(err)
}
if match != tc.match {
t.Fatalf("floatsMatchApprox: wrong result on %v", tc)
}
}
}
19 changes: 19 additions & 0 deletions pkg/testutils/floatcmp/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "floatcmp",
srcs = ["floatcmp.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/testutils/floatcmp",
visibility = ["//visibility:public"],
deps = [
"@com_github_google_go_cmp//cmp",
"@com_github_google_go_cmp//cmp/cmpopts",
],
)

go_test(
name = "floatcmp_test",
size = "small",
srcs = ["floatcmp_test.go"],
embed = [":floatcmp"],
)
77 changes: 77 additions & 0 deletions pkg/testutils/floatcmp/floatcmp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2021 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

// Package floatcmp provides functions for determining float values to be equal
// if they are within a tolerance. It is designed to be used in tests.
package floatcmp

import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
)

const (
// CloseFraction can be used to set a "close" tolerance for the fraction
// argument of functions in this package. It should typically be used with
// the CloseMargin constant for the margin argument. Its value is taken from
// the close tolerances in go's math package.
CloseFraction float64 = 1e-14

// CloseMargin can be used to set a "close" tolerance for the margin
// argument of functions in this package. It should typically be used with
// the CloseFraction constant for the fraction argument.
//
// It is set to the square of CloseFraction so it is only used when the
// smaller of the absolute expected and actual values is in the range:
//
// -CloseFraction <= 0 <= CloseFraction
//
// CloseMargin is greater than 0 otherwise if either expected or actual were
// 0 the calculated tolerance from the fraction would be 0.
CloseMargin float64 = CloseFraction * CloseFraction
)

// EqualApprox reports whether expected and actual are deeply equal with the
// following modifications for float64 and float32 types:
//
// • If both expected and actual are not NaN or infinate, they are equal within
// the larger of the relative fraction or absolute margin calculated from the
// fraction and margin arguments.
//
// • If both expected and actual are NaN, they are equal.
//
// Both fraction and margin must be non-negative.
//
// fraction is used to calculate the tolerance as a relative fraction of the
// smaller of expected and actual:
//
// tolerance_frac = (fraction * min(|expected|, |actual|))
//
// margin specifies the tolerance as an absolute value:
//
// tolerance_marg = margin
//
// The tolerance used to determine approximate equality is:
//
// tolerance = max(tolerance_frac, tolerance_marg)
//
// To use only one of fraction or margin, set the other to 0.
//
// For comparing expected and actual values in tests, typically the fraction
// should be set to the smallest relative fraction to tolerate. The margin
// should be set to a much smaller value so that it is only used when:
//
// (fraction * min(|expected|, |actual|)) < margin
//
// which allows expected and actual to be approximately equal within margin when
// either is 0.
func EqualApprox(expected interface{}, actual interface{}, fraction float64, margin float64) bool {
return cmp.Equal(expected, actual, cmpopts.EquateApprox(fraction, margin), cmpopts.EquateNaNs())
}
156 changes: 156 additions & 0 deletions pkg/testutils/floatcmp/floatcmp_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
// Copyright 2021 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package floatcmp

import (
"math"
"testing"
)

type (
// floatArgs holds the expected and actual values of floating point equality tests.
floatArgs struct {
expected float64
actual float64
}

// testStruct structs are the values compared in struct equality tests.
testStruct struct {
X, Y float64
I int
}

// structArgs holds the expected and actual values of struct equality tests.
structArgs struct {
expected testStruct
actual testStruct
}

// floatTestCase represents a test case for floating point values.
floatTestCase struct {
name string
args floatArgs
want bool
}

// structTestCase represents a test case for struct values.
structTestCase struct {
name string
args structArgs
want bool
}
)

var floatTests = []floatTestCase{
{
name: "zeros",
args: floatArgs{expected: 0, actual: 0},
want: true,
},
{
name: "NaNs",
args: floatArgs{expected: math.NaN(), actual: math.NaN()},
want: true,
},
{
name: "zero not close to NaN",
args: floatArgs{expected: 0, actual: math.NaN()},
want: false,
},
{
name: "positive infinities",
args: floatArgs{expected: math.Inf(+1), actual: math.Inf(+1)},
want: true,
},
{
name: "negative infinities",
args: floatArgs{expected: math.Inf(-1), actual: math.Inf(-1)},
want: true,
},
{
name: "ones",
args: floatArgs{expected: 1, actual: 1},
want: true,
},
{
name: "signs",
args: floatArgs{expected: 1, actual: -1},
want: false,
},
{
name: "different",
args: floatArgs{expected: 1, actual: 2},
want: false,
},
{
name: "close to zero",
args: floatArgs{expected: 0, actual: math.Nextafter(0+CloseMargin, math.Inf(-1))},
want: true,
},
{
name: "not close to zero",
args: floatArgs{expected: 0, actual: math.Nextafter(0+CloseMargin, math.Inf(+1))},
want: false,
},
{
name: "close to CloseFraction",
args: floatArgs{expected: CloseFraction, actual: math.Nextafter(CloseFraction+CloseMargin, math.Inf(-1))},
want: true,
},
{
name: "not close to CloseFraction",
args: floatArgs{expected: CloseFraction, actual: math.Nextafter(CloseFraction+CloseMargin, math.Inf(+1))},
want: false,
},
{
name: "close to one",
args: floatArgs{expected: 1, actual: math.Nextafter(1+1*CloseFraction, math.Inf(-1))},
want: true,
},
{
name: "not close to one",
args: floatArgs{expected: 1, actual: math.Nextafter(1+1*CloseFraction, math.Inf(+1))},
want: false,
},
}

// toStructTests transforms an array of floatTestsCases into an array of structTestCases by
// copying the expected and actual values of each floatTestCase into corresponding values
// in each structTestCase.
func toStructTests(floatTestCases []floatTestCase) []structTestCase {
structTestCases := make([]structTestCase, 0, len(floatTestCases))
for _, ft := range floatTestCases {
structTestCases = append(structTestCases,
structTestCase{
name: ft.name + " struct",
args: structArgs{expected: testStruct{X: ft.args.expected, Y: ft.args.expected, I: 0}, actual: testStruct{X: ft.args.actual, Y: ft.args.actual, I: 0}},
want: ft.want,
})
}
return structTestCases
}

func TestEqualClose(t *testing.T) {
for _, tt := range floatTests {
t.Run(tt.name, func(t *testing.T) {
if got := EqualApprox(tt.args.expected, tt.args.actual, CloseFraction, CloseMargin); got != tt.want {
t.Errorf("Close(%.16e, %.16e) = %v, want %v", tt.args.expected, tt.args.actual, got, tt.want)
}
})
}
for _, tt := range toStructTests(floatTests) {
t.Run(tt.name, func(t *testing.T) {
if got := EqualApprox(tt.args.expected, tt.args.actual, CloseFraction, CloseMargin); got != tt.want {
t.Errorf("Close(%.v, %.v) = %v, want %v", tt.args.expected, tt.args.actual, got, tt.want)
}
})
}
}

0 comments on commit b3237ed

Please sign in to comment.