Skip to content

Commit 175611c

Browse files
sephibsjmonson
authored andcommitted
fix: update duplicate percentile filtering to retain largest values for accuracy
Signed-off-by: Joseph Berry <joberry@redhat.com>
1 parent 9238934 commit 175611c

2 files changed

Lines changed: 21 additions & 16 deletions

File tree

src/guidellm/benchmark/outputs/html.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,27 +246,31 @@ def _filter_duplicate_percentiles(percentiles: dict[str, float]) -> dict[str, fl
246246
247247
When distributions have very few data points, multiple percentiles can have
248248
the same value, which causes visualization libraries to fail. This function
249-
keeps only the first occurrence of consecutive duplicate values.
249+
keeps only the largest percentile for consecutive duplicate values, which is
250+
more mathematically accurate as higher percentiles have greater statistical
251+
significance.
250252
251253
:param percentiles: Dictionary of percentile names to values
252254
:return: Filtered percentiles dictionary with no consecutive duplicates
253255
"""
254256
if not percentiles:
255257
return percentiles
256-
258+
257259
percentile_order = list(Percentiles.model_fields.keys())
258260

261+
# Iterate in reverse to keep the largest percentile for each value
259262
filtered = {}
260263
previous_value = None
261264

262-
for key in percentile_order:
265+
for key in reversed(percentile_order):
263266
if key in percentiles:
264267
current_value = percentiles[key]
265268
if previous_value is None or current_value != previous_value:
266269
filtered[key] = current_value
267270
previous_value = current_value
268271

269-
return filtered
272+
# Restore original order
273+
return {key: filtered[key] for key in percentile_order if key in filtered}
270274

271275

272276
def _inject_data(js_data: dict[str, str], html: str) -> str:

tests/unit/benchmark/test_html_output.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ def test_filter_all_same_values():
2121

2222
filtered = _filter_duplicate_percentiles(percentiles)
2323

24-
# Should only keep the first one
25-
assert filtered == {"p001": 15.288091352804853}
24+
# Should only keep the largest (p999) for mathematical accuracy
25+
assert filtered == {"p999": 15.288091352804853}
2626

2727

2828
def test_filter_consecutive_duplicates():
@@ -43,11 +43,11 @@ def test_filter_consecutive_duplicates():
4343

4444
filtered = _filter_duplicate_percentiles(percentiles)
4545

46-
# Should keep first of each group
46+
# Should keep largest of each group for mathematical accuracy
4747
assert filtered == {
48-
"p001": 15.288091352804853,
49-
"p50": 16.41327511776994,
50-
"p90": 17.03541629998259,
48+
"p25": 15.288091352804853,
49+
"p75": 16.41327511776994,
50+
"p999": 17.03541629998259,
5151
}
5252

5353

@@ -69,15 +69,16 @@ def test_no_duplicates():
6969

7070
filtered = _filter_duplicate_percentiles(percentiles)
7171

72+
# Should keep largest of each duplicate group (p01 instead of p001, p999 instead of p95)
7273
assert filtered == {
73-
"p001": 13.181080445834912,
74+
"p01": 13.181080445834912,
7475
"p05": 13.530595573836457,
7576
"p10": 13.843972502554365,
7677
"p25": 14.086376978251748,
7778
"p50": 14.403258051191058,
7879
"p75": 14.738608817056042,
7980
"p90": 15.18136631856698,
80-
"p95": 15.7213110894772,
81+
"p999": 15.7213110894772,
8182
}
8283

8384

@@ -150,11 +151,11 @@ def test_model_dump_filters_duplicates():
150151

151152
data = dist.model_dump()
152153

153-
# Check that percentiles were filtered
154+
# Check that percentiles were filtered, keeping largest of each group
154155
assert data["percentiles"] == {
155-
"p001": 15.288091352804853,
156-
"p50": 16.41327511776994,
157-
"p90": 17.03541629998259,
156+
"p25": 15.288091352804853,
157+
"p75": 16.41327511776994,
158+
"p999": 17.03541629998259,
158159
}
159160

160161
# Ensure other fields remain unchanged

0 commit comments

Comments
 (0)