From 8dab18a8a86b9582c4fcf9eefb4a38bf43edcec6 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sat, 22 Mar 2025 16:54:36 +0800 Subject: [PATCH 1/3] palette/moreland: fix floating-point arithmetic error in Palette Fixes https://github.com/gonum/plot/issues/798. Signed-off-by: Eng Zer Jun --- palette/moreland/luminance.go | 3 +-- palette/moreland/luminance_test.go | 8 ++++++++ palette/moreland/smooth.go | 2 +- palette/moreland/smooth_test.go | 8 ++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/palette/moreland/luminance.go b/palette/moreland/luminance.go index 0baced27..8fffa0e5 100644 --- a/palette/moreland/luminance.go +++ b/palette/moreland/luminance.go @@ -176,10 +176,9 @@ func (l luminance) Palette(n int) palette.Palette { l.SetMax(1) } delta := (l.max - l.min) / float64(n-1) - var v float64 c := make([]color.Color, n) for i := range n { - v = l.min + float64(delta*float64(i)) + v := min(l.min+float64(delta*float64(i)), l.max) var err error c[i], err = l.At(v) if err != nil { diff --git a/palette/moreland/luminance_test.go b/palette/moreland/luminance_test.go index a691c4df..5519bcfc 100644 --- a/palette/moreland/luminance_test.go +++ b/palette/moreland/luminance_test.go @@ -161,3 +161,11 @@ func BenchmarkLuminance_At(b *testing.B) { }) } } + +func Test_luminance_Palette(t *testing.T) { + // https://github.com/gonum/plot/issues/798 + colors := Kindlmann() + colors.SetMin(0.3402859786606234) + colors.SetMax(15.322841335211892) + colors.Palette(15) +} diff --git a/palette/moreland/smooth.go b/palette/moreland/smooth.go index 3b7057a3..6017c626 100644 --- a/palette/moreland/smooth.go +++ b/palette/moreland/smooth.go @@ -173,7 +173,7 @@ func (p smoothDiverging) Palette(n int) palette.Palette { delta := (p.max - p.min) / float64(n-1) c := make([]color.Color, n) for i := range c { - v := p.min + float64(delta*float64(i)) + v := min(p.min+float64(delta*float64(i)), p.max) var err error c[i], err = p.At(v) if err != nil { diff --git a/palette/moreland/smooth_test.go b/palette/moreland/smooth_test.go index ae128c69..771ceb6f 100644 --- a/palette/moreland/smooth_test.go +++ b/palette/moreland/smooth_test.go @@ -246,3 +246,11 @@ func similar(a, b color.Color, tolerance float64) bool { } return true } + +func Test_smoothDiverging_Palette(t *testing.T) { + // https://github.com/gonum/plot/issues/798 + colors := SmoothBlueRed() + colors.SetMin(0.3402859786606234) + colors.SetMax(15.322841335211892) + colors.Palette(15) +} From 55662b36b4613f9934453412090318e259a40308 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sat, 22 Mar 2025 18:22:21 +0800 Subject: [PATCH 2/3] Rename tests Reference: https://github.com/gonum/plot/pull/799#pullrequestreview-2707997213 Signed-off-by: Eng Zer Jun --- palette/moreland/luminance_test.go | 2 +- palette/moreland/smooth_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/palette/moreland/luminance_test.go b/palette/moreland/luminance_test.go index 5519bcfc..aede622f 100644 --- a/palette/moreland/luminance_test.go +++ b/palette/moreland/luminance_test.go @@ -162,7 +162,7 @@ func BenchmarkLuminance_At(b *testing.B) { } } -func Test_luminance_Palette(t *testing.T) { +func TestIssue798Kindlmann(t *testing.T) { // https://github.com/gonum/plot/issues/798 colors := Kindlmann() colors.SetMin(0.3402859786606234) diff --git a/palette/moreland/smooth_test.go b/palette/moreland/smooth_test.go index 771ceb6f..4d38ba59 100644 --- a/palette/moreland/smooth_test.go +++ b/palette/moreland/smooth_test.go @@ -247,7 +247,7 @@ func similar(a, b color.Color, tolerance float64) bool { return true } -func Test_smoothDiverging_Palette(t *testing.T) { +func TestIssue798SmoothBlueRed(t *testing.T) { // https://github.com/gonum/plot/issues/798 colors := SmoothBlueRed() colors.SetMin(0.3402859786606234) From 1434e666b2dbe032a84df62e813be71c9f6b8831 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sun, 23 Mar 2025 16:41:33 +0800 Subject: [PATCH 3/3] Apply code suggestions Reference: https://github.com/gonum/plot/pull/799#pullrequestreview-2708184852 Signed-off-by: Eng Zer Jun --- palette/moreland/luminance.go | 9 +++++- palette/moreland/luminance_test.go | 46 ++++++++++++++++++++++++++---- palette/moreland/smooth.go | 9 +++++- palette/moreland/smooth_test.go | 40 ++++++++++++++++++++++---- 4 files changed, 92 insertions(+), 12 deletions(-) diff --git a/palette/moreland/luminance.go b/palette/moreland/luminance.go index 8fffa0e5..001e6d3d 100644 --- a/palette/moreland/luminance.go +++ b/palette/moreland/luminance.go @@ -176,9 +176,16 @@ func (l luminance) Palette(n int) palette.Palette { l.SetMax(1) } delta := (l.max - l.min) / float64(n-1) + var v float64 c := make([]color.Color, n) for i := range n { - v := min(l.min+float64(delta*float64(i)), l.max) + if i == n-1 { + // Avoid potential overflow on last element + // due to floating point error. + v = l.max + } else { + v = l.min + float64(delta*float64(i)) + } var err error c[i], err = l.At(v) if err != nil { diff --git a/palette/moreland/luminance_test.go b/palette/moreland/luminance_test.go index aede622f..6e4c0312 100644 --- a/palette/moreland/luminance_test.go +++ b/palette/moreland/luminance_test.go @@ -162,10 +162,46 @@ func BenchmarkLuminance_At(b *testing.B) { } } +// See https://github.com/gonum/plot/issues/798 func TestIssue798Kindlmann(t *testing.T) { - // https://github.com/gonum/plot/issues/798 - colors := Kindlmann() - colors.SetMin(0.3402859786606234) - colors.SetMax(15.322841335211892) - colors.Palette(15) + for _, test := range []struct { + n int + min, max float64 + }{ + 0: {n: 2, min: 0, max: 1}, + 1: {n: 15, min: 0.3402859786606234, max: 15.322841335211892}, + } { + t.Run("", func(t *testing.T) { + defer func() { + r := recover() + if r != nil { + t.Errorf("unexpected panic with n=%d min=%f max=%f: %v", test.n, test.min, test.max, r) + } + }() + colors := Kindlmann() + colors.SetMin(test.min) + colors.SetMax(test.max) + col := colors.Palette(test.n).Colors() + min, err := colors.At(test.min) + if err != nil { + t.Fatalf("unexpected error calling colors.At(min): %v", err) + } + if !sameColor(min, col[0]) { + t.Errorf("unexpected min color %#v != %#v", min, col[0]) + } + max, err := colors.At(test.max) + if err != nil { + t.Fatalf("unexpected error calling colors.At(max): %v", err) + } + if !sameColor(max, col[len(col)-1]) { + t.Errorf("unexpected max color %#v != %#v", max, col[len(col)-1]) + } + }) + } +} + +func sameColor(a, b color.Color) bool { + ar, ag, ab, aa := a.RGBA() + br, bg, bb, ba := b.RGBA() + return ar == br && ag == bg && ab == bb && aa == ba } diff --git a/palette/moreland/smooth.go b/palette/moreland/smooth.go index 6017c626..427b6742 100644 --- a/palette/moreland/smooth.go +++ b/palette/moreland/smooth.go @@ -171,9 +171,16 @@ func (p smoothDiverging) Palette(n int) palette.Palette { p.SetMax(1) } delta := (p.max - p.min) / float64(n-1) + var v float64 c := make([]color.Color, n) for i := range c { - v := min(p.min+float64(delta*float64(i)), p.max) + if i == n-1 { + // Avoid potential overflow on last element + // due to floating point error. + v = p.max + } else { + v = p.min + float64(delta*float64(i)) + } var err error c[i], err = p.At(v) if err != nil { diff --git a/palette/moreland/smooth_test.go b/palette/moreland/smooth_test.go index 4d38ba59..06e7ab90 100644 --- a/palette/moreland/smooth_test.go +++ b/palette/moreland/smooth_test.go @@ -247,10 +247,40 @@ func similar(a, b color.Color, tolerance float64) bool { return true } +// See https://github.com/gonum/plot/issues/798 func TestIssue798SmoothBlueRed(t *testing.T) { - // https://github.com/gonum/plot/issues/798 - colors := SmoothBlueRed() - colors.SetMin(0.3402859786606234) - colors.SetMax(15.322841335211892) - colors.Palette(15) + for _, test := range []struct { + n int + min, max float64 + }{ + 0: {n: 2, min: 0, max: 1}, + 1: {n: 15, min: 0.3402859786606234, max: 15.322841335211892}, + } { + t.Run("", func(t *testing.T) { + defer func() { + r := recover() + if r != nil { + t.Errorf("unexpected panic with n=%d min=%f max=%f: %v", test.n, test.min, test.max, r) + } + }() + colors := SmoothBlueRed() + colors.SetMin(test.min) + colors.SetMax(test.max) + col := colors.Palette(test.n).Colors() + min, err := colors.At(test.min) + if err != nil { + t.Fatalf("unexpected error calling colors.At(min): %v", err) + } + if !sameColor(min, col[0]) { + t.Errorf("unexpected min color %#v != %#v", min, col[0]) + } + max, err := colors.At(test.max) + if err != nil { + t.Fatalf("unexpected error calling colors.At(max): %v", err) + } + if !sameColor(max, col[len(col)-1]) { + t.Errorf("unexpected max color %#v != %#v", max, col[len(col)-1]) + } + }) + } }