From d13062b81146ccb8a1f3b7d4d0f609a40c684eca Mon Sep 17 00:00:00 2001 From: AndrewDanilin Date: Tue, 12 Sep 2023 21:16:06 +0300 Subject: [PATCH 1/2] [#88] netmap: fix min aggregator bug, add tests Signed-off-by: Andrew Danilin --- netmap/aggregator.go | 12 +++++++---- netmap/aggregator_test.go | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 netmap/aggregator_test.go diff --git a/netmap/aggregator.go b/netmap/aggregator.go index c018e6b..3365fe3 100644 --- a/netmap/aggregator.go +++ b/netmap/aggregator.go @@ -23,7 +23,7 @@ type ( } minAgg struct { - min float64 + min *float64 } meanIQRAgg struct { @@ -102,13 +102,17 @@ func (a *meanAgg) Compute() float64 { } func (a *minAgg) Add(n float64) { - if a.min == 0 || n < a.min { - a.min = n + if a.min == nil || n < *a.min { + a.min = &n } } func (a *minAgg) Compute() float64 { - return a.min + if a.min == nil { + return 0 + } + + return *a.min } func (a *meanIQRAgg) Add(n float64) { diff --git a/netmap/aggregator_test.go b/netmap/aggregator_test.go new file mode 100644 index 0000000..d844803 --- /dev/null +++ b/netmap/aggregator_test.go @@ -0,0 +1,44 @@ +package netmap + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMinAgg(t *testing.T) { + tests := []struct { + vals []float64 + res float64 + }{ + { + vals: []float64{1, 2, 3, 0, 10}, + res: 0, + }, + { + vals: []float64{10, 0, 10, 0}, + res: 0, + }, + { + vals: []float64{0, 1, 2, 3, 10}, + res: 0, + }, + { + vals: []float64{0, 0, 0, 0}, + res: 0, + }, + { + vals: []float64{10, 10, 10, 10}, + res: 10, + }, + } + + for _, test := range tests { + a := newMinAgg() + for _, val := range test.vals { + a.Add(val) + } + + require.Equal(t, test.res, a.Compute()) + } +} -- 2.45.2 From c0780510aa5dad6fe666abeb95414d65acc73723 Mon Sep 17 00:00:00 2001 From: "andnilin@gmail.com" Date: Sat, 16 Sep 2023 17:03:38 +0300 Subject: [PATCH 2/2] [#88] netmap: use bool, fix hrw_sort tests Signed-off-by: Andrew Danilin --- netmap/aggregator.go | 19 +++++++++++-------- netmap/json_tests/hrw_sort.json | 8 ++++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/netmap/aggregator.go b/netmap/aggregator.go index 3365fe3..b2a9366 100644 --- a/netmap/aggregator.go +++ b/netmap/aggregator.go @@ -23,7 +23,8 @@ type ( } minAgg struct { - min *float64 + min float64 + minFound bool } meanIQRAgg struct { @@ -102,17 +103,19 @@ func (a *meanAgg) Compute() float64 { } func (a *minAgg) Add(n float64) { - if a.min == nil || n < *a.min { - a.min = &n + if !a.minFound { + a.min = n + a.minFound = true + return + } + + if n < a.min { + a.min = n } } func (a *minAgg) Compute() float64 { - if a.min == nil { - return 0 - } - - return *a.min + return a.min } func (a *meanIQRAgg) Add(n float64) { diff --git a/netmap/json_tests/hrw_sort.json b/netmap/json_tests/hrw_sort.json index 6ebeffc..8110ac7 100644 --- a/netmap/json_tests/hrw_sort.json +++ b/netmap/json_tests/hrw_sort.json @@ -146,19 +146,19 @@ "select 3 nodes in 3 distinct countries, same placement": { "policy": {"replicas":[{"count":1,"selector":"Main"}],"containerBackupFactor":1,"selectors":[{"name":"Main","count":3,"clause":"DISTINCT","attribute":"Country","filter":"*"}],"filters":[]}, "pivot": "Y29udGFpbmVySUQ=", - "result": [[4, 0, 7]], + "result": [[0, 2, 3]], "placement": { "pivot": "b2JqZWN0SUQ=", - "result": [[4, 0, 7]] + "result": [[0, 2, 3]] } }, "select 6 nodes in 3 distinct countries, different placement": { "policy": {"replicas":[{"count":1,"selector":"Main"}],"containerBackupFactor":2,"selectors":[{"name":"Main","count":3,"clause":"DISTINCT","attribute":"Country","filter":"*"}],"filters":[]}, "pivot": "Y29udGFpbmVySUQ=", - "result": [[4, 3, 0, 1, 7, 2]], + "result": [[0, 1, 2, 6, 3, 4]], "placement": { "pivot": "b2JqZWN0SUQ=", - "result": [[4, 3, 0, 7, 2, 1]] + "result": [[0, 1, 2, 6, 3, 4]] } } } -- 2.45.2