goodhistogram: base error bound on true worst-case bucket error

README.md

@@ -57,13 +57,38 @@ go get github.com/cockroachdb/goodhistogram
 h := goodhistogram.New(goodhistogram.Params{
     Lo:           500,    // lower bound of tracked range (e.g. 500ns)
     Hi:           60e9,   // upper bound (e.g. 60s in nanoseconds)
-    ErrorBound: 0.10,   // 10% relative error → schema 2
+    ErrorBound: 0.10,   // 10% worst-case relative error → schema 3
 })
 
 h.Record(1_500_000)  // 1.5ms
 h.Record(42_000)     // 42µs
 ```
 
+`ErrorBound` is the *worst-case* relative error of a reported quantile: any
+estimate is guaranteed to be within this fraction of the true value. See
+[Error bounds](#error-bounds) for what this guarantees and why it differs from
+DDSketch's relative accuracy.
+
+### Resolution presets
+
+If you don't have a specific range in mind, the resolution presets cover the
+full recordable range `[1, math.MaxInt64]` and differ only in accuracy and
+memory. Pick by the fidelity you need:
+
+| Preset | Schema | Worst-case error | Buckets | Memory / histogram |
+|---|---:|---:|---:|---:|
+| `CoarseParams` | 1 | ~41.4% | 126 | ~1 KB |
+| `StandardParams` | 2 | ~18.9% | 252 | ~2 KB |
+| `FineParams` | 3 | ~9.05% | 504 | ~4 KB |
+
+```go
+h := goodhistogram.New(goodhistogram.StandardParams)
+```
+
+Memory is dominated by the per-histogram counts array; the boundary and lookup
+tables live in a single shared, cached config, so allocating many histograms
+from the same preset does not multiply that overhead.
+
 ### Take a snapshot and compute quantiles
 
 ```go
@@ -151,9 +176,11 @@ What makes it different from these libraries is the following:
 goodhistogram uses the same exponential bucket scheme as Prometheus
 native histograms. For a given schema *s*, each power-of-two octave is
 divided into 2^*s* buckets with boundaries at 2^(*j*/2^*s*). The
-schema is chosen as the coarsest one whose relative error
-(*gamma* - 1)/(*gamma* + 1) is at or below the requested `ErrorBound`,
-where *gamma* = 2^(2^(-*s*)).
+schema is chosen as the coarsest one whose worst-case relative error
+*gamma* - 1 is at or below the requested `ErrorBound`,
+where *gamma* = 2^(2^(-*s*)). See [Error bounds](#error-bounds) for why the
+bound is *gamma* - 1 rather than the (*gamma* - 1)/(*gamma* + 1) figure used by
+DDSketch.
 
 At construction, a fixed array of `atomic.Uint64` counters is allocated
 covering the range `[Lo, Hi]`. The number of buckets is determined by
@@ -197,6 +224,35 @@ This produces more accurate estimates when the true density is not uniform
 within a bucket, which is nearly always the case for real-world distributions.
 This approach is described well in the base2histogram link cited above.
 
+### Error bounds
+
+`ErrorBound` is the maximum relative error of a reported quantile: for a true
+value *v*, the estimate is within `ErrorBound` × *v* of *v*.
+
+This is a stronger guarantee than the one DDSketch's relative accuracy provides,
+and it is worth being precise about the difference. For a bucket spanning
+[*b*, *gamma*·*b*], DDSketch reports the bucket's midpoint *m* = 2·*gamma*·*b* /
+(*gamma* + 1), which is equidistant in relative terms from both edges, so its
+error is at most (*gamma* - 1)/(*gamma* + 1). DDSketch can rely on that figure
+*because* it always reports the midpoint.
+
+goodhistogram does not report the midpoint. Its trapezoidal estimator uses the
+shape of the observed distribution to interpolate a value that can fall anywhere
+in [*b*, *gamma*·*b*], edges included. The worst case is a value whose true
+location is the bucket start *b* but which is reported at the end *gamma*·*b*:
+the relative error is then (*gamma*·*b* - *b*)/*b* = *gamma* - 1 — roughly twice
+the midpoint figure.
+
+Earlier versions selected the schema using the midpoint figure, so a histogram
+configured for, say, 10% error used schema 2 (midpoint error 8.6%) while its
+reported quantiles could drift up to *gamma* - 1 = 18.9% — visibly violating the
+configured bound
+([#9](https://github.com/cockroachdb/goodhistogram/issues/9)). Schema selection
+now uses *gamma* - 1, so the configured `ErrorBound` is the bound quantiles
+actually honor. The practical effect is that a given `ErrorBound` now selects a
+finer schema than before (10% now picks schema 3, not schema 2), trading a
+modest amount of extra memory for a guarantee that holds.
+
 ### Sacrifices in Accuracy
 
 As a final note on the design, goodhistogram takes two tracks which

benchmark_test.go

@@ -30,10 +30,14 @@ const (
 	benchSchema     int32   = 2
 	benchLo         float64 = 500
 	benchHi         float64 = 6e10
-	benchErrBound           = 0.10 // 10% relative error → schema 2
-	promBucketCount         = 60   // CockroachDB's standard bucket count
+	promBucketCount         = 60 // CockroachDB's standard bucket count
 )
 
+// benchErrBound is the error bound that resolves to benchSchema. It is derived
+// from the schema's worst-case relative error (γ-1) so the benchmarks stay
+// pinned to schema 2 regardless of the exact bound arithmetic.
+var benchErrBound = schemaRelativeError(benchSchema)
+
 // benchRange is benchHi - benchLo, used by samplers.
 const benchRange = benchHi - benchLo
 

histogram.go

@@ -61,11 +61,14 @@ func init() {
 	}
 }
 
-// schemaRelativeError returns the relative error for a given schema.
-// The error is (γ-1)/(γ+1) where γ = 2^(2^(-schema)).
+// schemaRelativeError returns the worst-case relative error for a schema:
+// γ-1 where γ = 2^(2^(-schema)). This is DDSketch's midpoint error
+// (γ-1)/(γ+1) doubled, because ValueAtQuantile reports anywhere in a bucket
+// [b, γ·b], not just the midpoint, so a value at b can be reported at γ·b. See
+// https://github.com/cockroachdb/goodhistogram/issues/9.
 func schemaRelativeError(schema int32) float64 {
 	gamma := math.Pow(2, math.Pow(2, float64(-schema)))
-	return (gamma - 1) / (gamma + 1)
+	return gamma - 1
 }
 
 // pickSchema selects the coarsest (fewest buckets) Prometheus schema whose
@@ -167,10 +170,10 @@ func newConfig(lo, hi, desiredError float64) config {
 	// values always round up to the next bucket — at most one
 	// bucket of additional error for a small fraction of values.
 	//
-	// For the common schema 2 (10% error), only 4 of 256 entries
-	// straddle, affecting ~0.4% of recorded values. The maximum
+	// For the common schema 2 (γ-1 = 18.9% worst-case error), only 4 of
+	// 256 entries straddle, affecting ~0.4% of recorded values. The maximum
 	// additional error for those values is bounded by one bucket width
-	// (8.6%), but the impact on quantile estimation is negligible since
+	// (γ-1), but the impact on quantile estimation is negligible since
 	// the affected values are already near the boundary.
 	var bucketLookup [bucketLookupSize]uint8
 	for tableIdx := 0; tableIdx < bucketLookupSize; tableIdx++ {
@@ -204,7 +207,7 @@ func newConfig(lo, hi, desiredError float64) config {
 // Zero-value fields are replaced with defaults:
 //   - Lo: 1
 //   - Hi: math.MaxInt64
-//   - ErrorBound: 0.10 (10%, schema 2)
+//   - ErrorBound: 0.10 (10%, schema 3)
 type Params struct {
 	// Lo and Hi define the tracked value range. Values outside this range
 	// are counted in Underflow/Overflow.
@@ -215,6 +218,32 @@ type Params struct {
 	ErrorBound float64
 }
 
+// Resolution presets cover the full range [1, math.MaxInt64] and differ only
+// in accuracy and memory — pick by the fidelity you need. Memory is the
+// per-histogram counts array; the config (boundaries, lookup table) is shared.
+var (
+	// CoarseParams: schema 1, ~41.4% error, 126 buckets, ~1 KB/histogram.
+	CoarseParams = Params{
+		Lo:         1,
+		Hi:         float64(math.MaxInt64),
+		ErrorBound: schemaRelativeError(1),
+	}
+
+	// StandardParams: schema 2, ~18.9% error, 252 buckets, ~2 KB/histogram.
+	StandardParams = Params{
+		Lo:         1,
+		Hi:         float64(math.MaxInt64),
+		ErrorBound: schemaRelativeError(2),
+	}
+
+	// FineParams: schema 3, ~9.05% error, 504 buckets, ~4 KB/histogram.
+	FineParams = Params{
+		Lo:         1,
+		Hi:         float64(math.MaxInt64),
+		ErrorBound: schemaRelativeError(3),
+	}
+)
+
 // Common Params presets, modeled after the bucket tiers in CockroachDB's
 // pkg/util/metric/histogram_buckets.go and Prometheus DefBuckets.
 //

histogram_test.go

@@ -24,15 +24,19 @@ func TestPickSchema(t *testing.T) {
 		desiredError float64
 		wantSchema   int32
 	}{
-		{0.35, 0},  // 33.3% error for schema 0
-		{0.10, 2},  // 8.6% error for schema 2
-		{0.05, 3},  // 4.3% error for schema 3
-		{0.03, 4},  // 2.17% error for schema 4
-		{0.02, 5},  // schema 4 is 2.17% > 2%, so need schema 5 (1.08%)
-		{0.015, 5}, // 1.08% for schema 5
-		{0.005, 7}, // schema 6 is 0.54% > 0.5%, so need schema 7 (0.27%)
-		{0.003, 7}, // 0.27% for schema 7
-		{0.002, 8}, // 0.14% for schema 8
+		// Worst-case relative error per schema is γ-1: schema 0 100%,
+		// 1 41.4%, 2 18.9%, 3 9.05%, 4 4.43%, 5 2.19%, 6 1.09%, 7 0.54%,
+		// 8 0.27%. pickSchema returns the coarsest schema at or below the
+		// requested bound.
+		{0.35, 2},  // schema 1 is 41.4% > 35%, so need schema 2 (18.9%)
+		{0.10, 3},  // schema 2 is 18.9% > 10%, so need schema 3 (9.05%)
+		{0.05, 4},  // schema 3 is 9.05% > 5%, so need schema 4 (4.43%)
+		{0.03, 5},  // schema 4 is 4.43% > 3%, so need schema 5 (2.19%)
+		{0.02, 6},  // schema 5 is 2.19% > 2%, so need schema 6 (1.09%)
+		{0.015, 6}, // 1.09% for schema 6
+		{0.005, 8}, // schema 7 is 0.54% > 0.5%, so need schema 8 (0.27%)
+		{0.003, 8}, // 0.27% for schema 8
+		{0.002, 8}, // schema 8 is finest available, though 0.27% > 0.2%
 		{0.001, 8}, // still schema 8 (finest available)
 	}
 	for _, tt := range tests {
@@ -51,10 +55,37 @@ func TestPickSchema(t *testing.T) {
 	}
 }
 
+func TestResolutionPresets(t *testing.T) {
+	tests := []struct {
+		name       string
+		params     Params
+		wantSchema int32
+		wantBucket int // full-range bucket count
+	}{
+		{"Coarse", CoarseParams, 1, 126},
+		{"Standard", StandardParams, 2, 252},
+		{"Fine", FineParams, 3, 504},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Presets span the full recordable range.
+			require.Equal(t, float64(1), tt.params.Lo)
+			require.Equal(t, float64(math.MaxInt64), tt.params.Hi)
+
+			h := New(tt.params)
+			require.Equal(t, tt.wantSchema, h.Schema())
+			require.Equal(t, tt.wantBucket, h.cfg.numBuckets)
+
+			// ErrorBound must equal the schema's worst-case error (γ-1).
+			require.InEpsilon(t, schemaRelativeError(tt.wantSchema), tt.params.ErrorBound, 1e-12)
+		})
+	}
+}
+
 func TestNewConfig(t *testing.T) {
 	t.Run("basic", func(t *testing.T) {
 		cfg := newConfig(1e4, 1e16, 0.05)
-		require.Equal(t, int32(3), cfg.schema) // 4.3% error
+		require.Equal(t, int32(4), cfg.schema) // 4.43% worst-case error
 		require.Greater(t, cfg.numBuckets, 0)
 		require.Equal(t, cfg.numBuckets+1, len(cfg.boundaries))
 		require.Equal(t, 1e4, cfg.boundaries[0])

windowed_test.go

@@ -351,7 +351,7 @@ func TestWindowedSnapshotConcurrentTickNoUnderflow(t *testing.T) {
 
 func TestWindowedSchema(t *testing.T) {
 	w := NewWindowed(Params{Lo: 1, Hi: 1e6, ErrorBound: 0.05}, time.Hour)
-	require.Equal(t, int32(3), w.Schema())
+	require.Equal(t, int32(4), w.Schema())
 }
 
 func TestNewWindowedPanicsOnZeroWindow(t *testing.T) {