Add fixed-point `WithinTolerance` method and greater-than operators #1135

ffranr · 2024-09-24T15:27:25Z

This pull request introduces the following changes to the rfqmath library:

FixedPoint[T].WithinTolerance method
Adds the WithinTolerance method to the FixedPoint[T] type, allowing two fixed-point numbers to be checked for equality within a specified tolerance in Parts Per Million (PPM).
Greater-than operators for integer types
Adds support for handling the > and >= comparisons for integer types.

These changes are necessary towards closing #871.

This commit introduces functions to handle the > and >= comparisons for integer types.

coveralls · 2024-09-24T15:36:25Z

Pull Request Test Coverage Report for Build 11055315406

Details

40 of 46 (86.96%) changed or added relevant lines in 2 files are covered.
8 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.05%) to 40.444%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rfqmath/arithmetic.go	6	12	50.0%

Files with Coverage Reduction	New Missed Lines	%
universe/interface.go	4	50.22%
tapgarden/caretaker.go	4	68.87%

Totals
Change from base Build 11038470643:	0.05%
Covered Lines:	24295
Relevant Lines:	60070

💛 - Coveralls

rfqmath/fixed_point.go

rfqmath/fixed_point_test.go

dstadulis · 2024-09-24T17:23:16Z

rfqmath/fixed_point_test.go

+			firstFp:      FixedPointFromUint64[N](1000000, 1),
+			secondFp:     FixedPointFromUint64[N](900000, 1),


Are there any overflow cases to handle?
Seems there should be sufficient space:

log(2**64)/log(1000000*900000) = 1.6

WithinTolerance uses methods from the rfqmath.Arithmetic interface for calculations, so I think the overflow responsibility should be on those methods. I'm not sure if its possible to get the Mul calls to overflow.

I'm not sure if its possible to get the Mul calls to overflow.

So we'll use the BigInt[T] version everywhere, which for all practical purposes, can't actually overflow.

dstadulis · 2024-09-24T18:38:44Z

Content

PriceOracle will deliver quote
Then internal checks will be done

fixedPoints

Is a simplification
in the RFQ quotes, the conversion rate between two assets right now there's two different fields, both assets are compared to BTC, which could be a big number
fixedPoints separate the scalar / separate the significant, can transport in a wireMessage without uint64

rfqmath/fixed_point.go

rfq/negotiator.go

rfqmath/fixed_point_test.go

ffranr · 2024-09-25T11:07:49Z

Use case for this new method is replacing pricesWithinBounds, see:
https://github.com/lightninglabs/taproot-assets/pull/1135/files#diff-64a27caf98eee653f23ffa833568a598241835b52d96fa4e26fc4127f92ad4f3R522

gijswijs

Nice looking PR. Made some comments throughout.

I'm still not fully understanding the use case tho:

This will be used during the handling of sell accept messages and buy accept messages that the node receives from its peer. The bid price or the ask price in those messages is compared to the bid price or the ask price respectively that our oracle provides. If it's within bounds (specified in ppm) we accept the offer.

But iiuc at that point we are already comparing prices converted to BTC? But the WithinTolerance works with FixedPoint[T] which I very much associate with taproot assets.

Shouldn't WithinTolerance work with lnwire.MilliSatoshi parameters?

rfqmath/fixed_point_test.go

gijswijs · 2024-09-25T14:59:22Z

rfqmath/fixed_point_test.go

@@ -175,4 +338,6 @@ func TestFixedPoint(t *testing.T) {
 	t.Run("equality", rapid.MakeCheck(testEquality[BigInt]))

 	t.Run("from_uint64", rapid.MakeCheck(testFromUint64[BigInt]))
+
+	t.Run("within_tolerance", rapid.MakeCheck(testWithinTolerance[BigInt]))


Why are we using rapid here if we don't use pseudo-random data for the inputs?

Just following the existing pattern. I might add random data inputs if I can think of something useful.

If you want to use rapid, then the tests need to be written in a more black-box style. So asserting some invariants/properties based on the desired behavior of the function in the abstract.

Non-blocking though.

I think we can retain this current TDD test (moving it outside the scope of this function), and also add this property based test to supplement things:

package rfqmath import ( "math/big" "testing" "github.com/stretchr/testify/require" "pgregory.net/rapid" ) func TestWithinToleranceBigInt(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Create a require instance req := require.New(t) // Generate random BigInts and scales a := rapid.Uint64().Draw(t, "a") b := rapid.Uint64().Draw(t, "b") scaleA := rapid.Uint8Range(0, 18).Draw(t, "scaleA") scaleB := rapid.Uint8Range(0, 18).Draw(t, "scaleB") tolerancePpm := rapid.Uint64Range(0, 1000000).Draw(t, "tolerancePpm") fpA := FixedPointFromUint64[BigInt](a, scaleA) fpB := FixedPointFromUint64[BigInt](b, scaleB) // Test property: WithinTolerance should be reflexive req.True(fpA.WithinTolerance(fpA, NewBigInt(big.NewInt(int64(tolerancePpm)))), "A number should be within tolerance of itself") // Test property: WithinTolerance should be symmetric resultAB := fpA.WithinTolerance(fpB, NewBigInt(big.NewInt(int64(tolerancePpm)))) resultBA := fpB.WithinTolerance(fpA, NewBigInt(big.NewInt(int64(tolerancePpm)))) req.Equal(resultAB, resultBA, "WithinTolerance should be symmetric") // Test property: Zero tolerance should only allow exact equality zeroTolerance := NewBigInt(big.NewInt(0)) req.Equal(fpA.WithinTolerance(fpB, zeroTolerance), fpA.Equals(fpB), "Zero tolerance should only allow exact equality") // Test property: Maximum tolerance (1,000,000 PPM) should always return true maxTolerance := NewBigInt(big.NewInt(1000000)) req.True(fpA.WithinTolerance(fpB, maxTolerance), "Maximum tolerance should always return true") // Test property: Tolerance behavior near boundaries if !fpA.Equals(fpB) { largerScale := scaleA if scaleB > scaleA { largerScale = scaleB } fpAScaled := fpA.ScaleTo(largerScale) fpBScaled := fpB.ScaleTo(largerScale) var delta *big.Int if fpAScaled.Coefficient.Gt(fpBScaled.Coefficient) { delta = new(big.Int).Sub(fpAScaled.Coefficient.value, fpBScaled.Coefficient.value) } else { delta = new(big.Int).Sub(fpBScaled.Coefficient.value, fpAScaled.Coefficient.value) } maxCoeff := new(big.Int).Set(fpAScaled.Coefficient.value) if fpBScaled.Coefficient.Gt(fpAScaled.Coefficient) { maxCoeff.Set(fpBScaled.Coefficient.value) } // Calculate the exact PPM difference ppmDiff := new(big.Int).Mul(delta, big.NewInt(1000000)) ppmDiff.Div(ppmDiff, maxCoeff) // Test that WithinTolerance returns true for tolerances greater than or equal to ppmDiff req.True(fpA.WithinTolerance(fpB, NewBigInt(new(big.Int).Add(ppmDiff, big.NewInt(1)))), "Should be within tolerance for tolerance > ppmDiff") // Test that WithinTolerance returns false for tolerances less than ppmDiff if ppmDiff.Sign() > 0 { req.False(fpA.WithinTolerance(fpB, NewBigInt(new(big.Int).Sub(ppmDiff, big.NewInt(1)))), "Should not be within tolerance for tolerance < ppmDiff") } } }) }

A variant that breaks things up a bit, and uses float64 to compute the final tolerance values directly:

package rfqmath_test import ( "math" "math/big" "testing" "pgregory.net/rapid" "github.com/yourusername/yourmodule/rfqmath" ) func TestWithinTolerance_EqualValues(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate a random coefficient and scale coef := t.Int64() scale := uint8(t.Uint32Range(0, 18)) tolerancePpm := t.Uint64() // Create two identical FixedPoint[BigInt] values f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef)), Scale: scale, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef)), Scale: scale, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) // The result should always be true when the values are equal result := f1.WithinTolerance(f2, tolerancePpmBigInt) if !result { t.Fatalf("WithinTolerance should be true when values are equal, but got false") } }) } func TestWithinTolerance_ToleranceZero(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients and scales coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(0)) result := f1.WithinTolerance(f2, tolerancePpmBigInt) // Scale both to the larger scale for comparison largerScale := scale1 if scale2 > scale1 { largerScale = scale2 } scaledF1 := f1.ScaleTo(largerScale) scaledF2 := f2.ScaleTo(largerScale) equal := scaledF1.Coefficient.Equals(scaledF2.Coefficient) if result != equal { t.Fatalf("WithinTolerance with zero tolerance mismatch: expected %v, got %v", equal, result) } }) } func TestWithinTolerance_Symmetric(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients, scales, and tolerance coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) tolerancePpm := t.Uint64() f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) result1 := f1.WithinTolerance(f2, tolerancePpmBigInt) result2 := f2.WithinTolerance(f1, tolerancePpmBigInt) if result1 != result2 { t.Fatalf("WithinTolerance is not symmetric: f1.WithinTolerance(f2)=%v, f2.WithinTolerance(f1)=%v", result1, result2) } }) } func TestWithinTolerance_MaxTolerance(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients and scales coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) // Set tolerancePpm to a large value tolerancePpm := t.Uint64Range(1_000_000, 10_000_000_000) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) // The result should always be true when tolerancePpm is large result := f1.WithinTolerance(f2, tolerancePpmBigInt) if !result { t.Fatalf("WithinTolerance should be true when tolerancePpm is large, but got false") } }) } func TestWithinTolerance_Property(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients within a reasonable range coef1 := t.Int64Range(-1_000_000_000, 1_000_000_000) coef2 := t.Int64Range(-1_000_000_000, 1_000_000_000) scale1 := uint8(t.Uint32Range(0, 9)) scale2 := uint8(t.Uint32Range(0, 9)) tolerancePpm := t.Uint64Range(0, 1_000_000) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) result := f1.WithinTolerance(f2, tolerancePpmBigInt) // Compute expected result using float64 f1Float := f1.ToFloat64() f2Float := f2.ToFloat64() delta := math.Abs(f1Float - f2Float) maxVal := math.Max(math.Abs(f1Float), math.Abs(f2Float)) tolerance := (float64(tolerancePpm) / 1_000_000) * maxVal expected := delta <= tolerance if result != expected { t.Fatalf("WithinTolerance mismatch:\n"+ "f1 = %v (float: %f),\n"+ "f2 = %v (float: %f),\n"+ "tolerancePpm = %v,\n"+ "delta = %e,\n"+ "tolerance = %e,\n"+ "result = %v,\n"+ "expected = %v", f1, f1Float, f2, f2Float, tolerancePpm, delta, tolerance, result, expected) } }) }

rfqmath/fixed_point_test.go

ffranr · 2024-09-25T16:30:13Z

Nice looking PR. Made some comments throughout.

I'm still not fully understanding the use case tho:

This will be used during the handling of sell accept messages and buy accept messages that the node receives from its peer. The bid price or the ask price in those messages is compared to the bid price or the ask price respectively that our oracle provides. If it's within bounds (specified in ppm) we accept the offer.

But iiuc at that point we are already comparing prices converted to BTC? But the WithinTolerance works with FixedPoint[T] which I very much associate with taproot assets.

Shouldn't WithinTolerance work with lnwire.MilliSatoshi parameters?

@gijswijs Thanks for taking a look.

This PR is in preparation for a follow-up PR. In the follow-up, the price oracle and RFQ wire messages will communicate asset-to-BTC rates instead of BTC prices. The current setup is a workaround where the rate coefficient is effectively typed as a millisatoshi amount. The follow-up PR will address and correct this.

Basically we want to compare rates and not prices. And the "prices" we're currently dealing with are rates in disguise.

Roasbeef

LGTM 🎽

Roasbeef · 2024-09-26T05:19:02Z

rfqmath/fixed_point_test.go

+			firstFp:      FixedPointFromUint64[N](1000000, 1),
+			secondFp:     FixedPointFromUint64[N](900000, 1),


I'm not sure if its possible to get the Mul calls to overflow.

So we'll use the BigInt[T] version everywhere, which for all practical purposes, can't actually overflow.

Roasbeef · 2024-09-26T05:21:20Z

rfqmath/fixed_point_test.go

@@ -175,4 +338,6 @@ func TestFixedPoint(t *testing.T) {
 	t.Run("equality", rapid.MakeCheck(testEquality[BigInt]))

 	t.Run("from_uint64", rapid.MakeCheck(testFromUint64[BigInt]))
+
+	t.Run("within_tolerance", rapid.MakeCheck(testWithinTolerance[BigInt]))


If you want to use rapid, then the tests need to be written in a more black-box style. So asserting some invariants/properties based on the desired behavior of the function in the abstract.

Non-blocking though.

Roasbeef · 2024-09-26T05:40:34Z

rfqmath/fixed_point_test.go

@@ -175,4 +338,6 @@ func TestFixedPoint(t *testing.T) {
 	t.Run("equality", rapid.MakeCheck(testEquality[BigInt]))

 	t.Run("from_uint64", rapid.MakeCheck(testFromUint64[BigInt]))
+
+	t.Run("within_tolerance", rapid.MakeCheck(testWithinTolerance[BigInt]))


I think we can retain this current TDD test (moving it outside the scope of this function), and also add this property based test to supplement things:

package rfqmath import ( "math/big" "testing" "github.com/stretchr/testify/require" "pgregory.net/rapid" ) func TestWithinToleranceBigInt(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Create a require instance req := require.New(t) // Generate random BigInts and scales a := rapid.Uint64().Draw(t, "a") b := rapid.Uint64().Draw(t, "b") scaleA := rapid.Uint8Range(0, 18).Draw(t, "scaleA") scaleB := rapid.Uint8Range(0, 18).Draw(t, "scaleB") tolerancePpm := rapid.Uint64Range(0, 1000000).Draw(t, "tolerancePpm") fpA := FixedPointFromUint64[BigInt](a, scaleA) fpB := FixedPointFromUint64[BigInt](b, scaleB) // Test property: WithinTolerance should be reflexive req.True(fpA.WithinTolerance(fpA, NewBigInt(big.NewInt(int64(tolerancePpm)))), "A number should be within tolerance of itself") // Test property: WithinTolerance should be symmetric resultAB := fpA.WithinTolerance(fpB, NewBigInt(big.NewInt(int64(tolerancePpm)))) resultBA := fpB.WithinTolerance(fpA, NewBigInt(big.NewInt(int64(tolerancePpm)))) req.Equal(resultAB, resultBA, "WithinTolerance should be symmetric") // Test property: Zero tolerance should only allow exact equality zeroTolerance := NewBigInt(big.NewInt(0)) req.Equal(fpA.WithinTolerance(fpB, zeroTolerance), fpA.Equals(fpB), "Zero tolerance should only allow exact equality") // Test property: Maximum tolerance (1,000,000 PPM) should always return true maxTolerance := NewBigInt(big.NewInt(1000000)) req.True(fpA.WithinTolerance(fpB, maxTolerance), "Maximum tolerance should always return true") // Test property: Tolerance behavior near boundaries if !fpA.Equals(fpB) { largerScale := scaleA if scaleB > scaleA { largerScale = scaleB } fpAScaled := fpA.ScaleTo(largerScale) fpBScaled := fpB.ScaleTo(largerScale) var delta *big.Int if fpAScaled.Coefficient.Gt(fpBScaled.Coefficient) { delta = new(big.Int).Sub(fpAScaled.Coefficient.value, fpBScaled.Coefficient.value) } else { delta = new(big.Int).Sub(fpBScaled.Coefficient.value, fpAScaled.Coefficient.value) } maxCoeff := new(big.Int).Set(fpAScaled.Coefficient.value) if fpBScaled.Coefficient.Gt(fpAScaled.Coefficient) { maxCoeff.Set(fpBScaled.Coefficient.value) } // Calculate the exact PPM difference ppmDiff := new(big.Int).Mul(delta, big.NewInt(1000000)) ppmDiff.Div(ppmDiff, maxCoeff) // Test that WithinTolerance returns true for tolerances greater than or equal to ppmDiff req.True(fpA.WithinTolerance(fpB, NewBigInt(new(big.Int).Add(ppmDiff, big.NewInt(1)))), "Should be within tolerance for tolerance > ppmDiff") // Test that WithinTolerance returns false for tolerances less than ppmDiff if ppmDiff.Sign() > 0 { req.False(fpA.WithinTolerance(fpB, NewBigInt(new(big.Int).Sub(ppmDiff, big.NewInt(1)))), "Should not be within tolerance for tolerance < ppmDiff") } } }) }

Roasbeef · 2024-09-26T05:41:43Z

rfqmath/fixed_point_test.go

@@ -175,4 +338,6 @@ func TestFixedPoint(t *testing.T) {
 	t.Run("equality", rapid.MakeCheck(testEquality[BigInt]))

 	t.Run("from_uint64", rapid.MakeCheck(testFromUint64[BigInt]))
+
+	t.Run("within_tolerance", rapid.MakeCheck(testWithinTolerance[BigInt]))


A variant that breaks things up a bit, and uses float64 to compute the final tolerance values directly:

package rfqmath_test import ( "math" "math/big" "testing" "pgregory.net/rapid" "github.com/yourusername/yourmodule/rfqmath" ) func TestWithinTolerance_EqualValues(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate a random coefficient and scale coef := t.Int64() scale := uint8(t.Uint32Range(0, 18)) tolerancePpm := t.Uint64() // Create two identical FixedPoint[BigInt] values f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef)), Scale: scale, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef)), Scale: scale, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) // The result should always be true when the values are equal result := f1.WithinTolerance(f2, tolerancePpmBigInt) if !result { t.Fatalf("WithinTolerance should be true when values are equal, but got false") } }) } func TestWithinTolerance_ToleranceZero(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients and scales coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(0)) result := f1.WithinTolerance(f2, tolerancePpmBigInt) // Scale both to the larger scale for comparison largerScale := scale1 if scale2 > scale1 { largerScale = scale2 } scaledF1 := f1.ScaleTo(largerScale) scaledF2 := f2.ScaleTo(largerScale) equal := scaledF1.Coefficient.Equals(scaledF2.Coefficient) if result != equal { t.Fatalf("WithinTolerance with zero tolerance mismatch: expected %v, got %v", equal, result) } }) } func TestWithinTolerance_Symmetric(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients, scales, and tolerance coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) tolerancePpm := t.Uint64() f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) result1 := f1.WithinTolerance(f2, tolerancePpmBigInt) result2 := f2.WithinTolerance(f1, tolerancePpmBigInt) if result1 != result2 { t.Fatalf("WithinTolerance is not symmetric: f1.WithinTolerance(f2)=%v, f2.WithinTolerance(f1)=%v", result1, result2) } }) } func TestWithinTolerance_MaxTolerance(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients and scales coef1 := t.Int64() coef2 := t.Int64() scale1 := uint8(t.Uint32Range(0, 18)) scale2 := uint8(t.Uint32Range(0, 18)) // Set tolerancePpm to a large value tolerancePpm := t.Uint64Range(1_000_000, 10_000_000_000) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) // The result should always be true when tolerancePpm is large result := f1.WithinTolerance(f2, tolerancePpmBigInt) if !result { t.Fatalf("WithinTolerance should be true when tolerancePpm is large, but got false") } }) } func TestWithinTolerance_Property(t *testing.T) { rapid.Check(t, func(t *rapid.T) { // Generate random coefficients within a reasonable range coef1 := t.Int64Range(-1_000_000_000, 1_000_000_000) coef2 := t.Int64Range(-1_000_000_000, 1_000_000_000) scale1 := uint8(t.Uint32Range(0, 9)) scale2 := uint8(t.Uint32Range(0, 9)) tolerancePpm := t.Uint64Range(0, 1_000_000) f1 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef1)), Scale: scale1, } f2 := rfqmath.FixedPoint[rfqmath.BigInt]{ Coefficient: rfqmath.NewBigInt(big.NewInt(coef2)), Scale: scale2, } tolerancePpmBigInt := rfqmath.NewBigInt(big.NewInt(int64(tolerancePpm))) result := f1.WithinTolerance(f2, tolerancePpmBigInt) // Compute expected result using float64 f1Float := f1.ToFloat64() f2Float := f2.ToFloat64() delta := math.Abs(f1Float - f2Float) maxVal := math.Max(math.Abs(f1Float), math.Abs(f2Float)) tolerance := (float64(tolerancePpm) / 1_000_000) * maxVal expected := delta <= tolerance if result != expected { t.Fatalf("WithinTolerance mismatch:\n"+ "f1 = %v (float: %f),\n"+ "f2 = %v (float: %f),\n"+ "tolerancePpm = %v,\n"+ "delta = %e,\n"+ "tolerance = %e,\n"+ "result = %v,\n"+ "expected = %v", f1, f1Float, f2, f2Float, tolerancePpm, delta, tolerance, result, expected) } }) }

This commit introduces the WithinTolerance method to FixedPoint[T]. The method checks if two fixed-point numbers are within a given tolerance, specified in Parts Per Million (PPM), and returns true if they are.

ffranr · 2024-09-26T15:22:22Z

I've added the property based tests that roas suggested.

GeorgeTsagk

LGTM

GeorgeTsagk · 2024-09-27T15:34:30Z

rfqmath/fixed_point_test.go

+			},
+			secondFp: FixedPoint[N]{
+				Coefficient: NewInt[N]().FromUint64(
+					314_159_265_359,


rfqmath: add greater-than operators to integer types

867a72c

This commit introduces functions to handle the > and >= comparisons for integer types.

ffranr requested review from gijswijs and GeorgeTsagk September 24, 2024 15:27

ffranr self-assigned this Sep 24, 2024

dstadulis added this to the v0.4.2 milestone Sep 24, 2024

dstadulis added bLIP rfq labels Sep 24, 2024

dstadulis reviewed Sep 24, 2024

View reviewed changes

Roasbeef reviewed Sep 25, 2024

View reviewed changes

rfqmath/fixed_point.go Show resolved Hide resolved

Roasbeef requested changes Sep 25, 2024

View reviewed changes

rfqmath/fixed_point.go Show resolved Hide resolved

GeorgeTsagk reviewed Sep 25, 2024

View reviewed changes

rfq/negotiator.go Show resolved Hide resolved

rfqmath/fixed_point_test.go Outdated Show resolved Hide resolved

rfqmath/fixed_point_test.go Show resolved Hide resolved

ffranr requested review from Roasbeef and GeorgeTsagk September 25, 2024 11:04

ffranr force-pushed the fixed-point-within-tolerance branch from a3e7afc to dfa1c7f Compare September 25, 2024 11:05

ffranr requested a review from dstadulis September 25, 2024 11:05

gijswijs reviewed Sep 25, 2024

View reviewed changes

ffranr force-pushed the fixed-point-within-tolerance branch from dfa1c7f to 63273a1 Compare September 25, 2024 16:38

ffranr requested a review from gijswijs September 25, 2024 16:38

Roasbeef approved these changes Sep 26, 2024

View reviewed changes

rfqmath: add FixedPoint[T].WithinTolerance method

b078457

This commit introduces the WithinTolerance method to FixedPoint[T]. The method checks if two fixed-point numbers are within a given tolerance, specified in Parts Per Million (PPM), and returns true if they are.

ffranr force-pushed the fixed-point-within-tolerance branch from 63273a1 to b078457 Compare September 26, 2024 15:21

ffranr removed request for dstadulis and GeorgeTsagk September 27, 2024 12:37

GeorgeTsagk approved these changes Sep 27, 2024

View reviewed changes

ffranr added this pull request to the merge queue Sep 27, 2024

Merged via the queue into main with commit 656245d Sep 27, 2024
18 checks passed

guggero deleted the fixed-point-within-tolerance branch October 6, 2024 08:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fixed-point `WithinTolerance` method and greater-than operators #1135

Add fixed-point `WithinTolerance` method and greater-than operators #1135

ffranr commented Sep 24, 2024

coveralls commented Sep 24, 2024 •

edited

Loading

dstadulis Sep 24, 2024

ffranr Sep 25, 2024

Roasbeef Sep 26, 2024

dstadulis commented Sep 24, 2024

ffranr commented Sep 25, 2024

gijswijs left a comment

gijswijs Sep 25, 2024

ffranr Sep 25, 2024

Roasbeef Sep 26, 2024

Roasbeef Sep 26, 2024

Roasbeef Sep 26, 2024 •

edited

Loading

ffranr commented Sep 25, 2024 •

edited

Loading

Roasbeef left a comment

Roasbeef Sep 26, 2024

Roasbeef Sep 26, 2024

Roasbeef Sep 26, 2024

Roasbeef Sep 26, 2024 •

edited

Loading

ffranr commented Sep 26, 2024

GeorgeTsagk left a comment

GeorgeTsagk Sep 27, 2024

		firstFp: FixedPointFromUint64[N](1000000, 1),
		secondFp: FixedPointFromUint64[N](900000, 1),

Add fixed-point WithinTolerance method and greater-than operators #1135

Add fixed-point WithinTolerance method and greater-than operators #1135

Conversation

ffranr commented Sep 24, 2024

coveralls commented Sep 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11055315406

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstadulis commented Sep 24, 2024

Content

fixedPoints

ffranr commented Sep 25, 2024

gijswijs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

ffranr commented Sep 25, 2024 • edited Loading

Roasbeef left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

ffranr commented Sep 26, 2024

GeorgeTsagk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add fixed-point `WithinTolerance` method and greater-than operators #1135

Add fixed-point `WithinTolerance` method and greater-than operators #1135

coveralls commented Sep 24, 2024 •

edited

Loading

Roasbeef Sep 26, 2024 •

edited

Loading

ffranr commented Sep 25, 2024 •

edited

Loading

Roasbeef Sep 26, 2024 •

edited

Loading