Skip to content

Make aggregation function selection consistent across Run and Chart pages #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 3, 2025
Merged

Make aggregation function selection consistent across Run and Chart pages #42

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14d8fa2
Make aggregation function selection consistent across Run and Chart p…
ldionne Oct 2, 2025
6dafde9
Address review comments for 404 instead of assert
ldionne Oct 2, 2025
c72a973
Fix flake8 issues: GH action is working
ldionne Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lnt/server/ui/templates/v4_graph.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,18 @@ <h4>Controls</h4>
{{ 'checked="checked"' if options.xaxis_date else ""}}/></td>
</tr>
<tr>
<td>Mean() as Aggregation</td>
<td><input type="checkbox" name="switch_min_mean" value="yes"
{{ 'checked="checked"' if options.switch_min_mean else ""}}/></td>
<td>Aggregation function:</td>
</tr>
{# Split this into a new row to avoid making the dialog wider. #}
<tr>
<td>
<select name="aggregation_function" id="aggregation_function">
<option value="min" {{ 'selected="selected"' if options.aggregation_function == 'min' else '' }}>Minimum</option>
<option value="max" {{ 'selected="selected"' if options.aggregation_function == 'max' else '' }}>Maximum</option>
<option value="mean" {{ 'selected="selected"' if options.aggregation_function == 'mean' else '' }}>Mean</option>
<option value="median" {{ 'selected="selected"' if options.aggregation_function == 'median' else '' }}>Median</option>
</select>
</td>
</tr>
<tr>
<td>Hide Line Plot:</td>
Expand Down
12 changes: 6 additions & 6 deletions lnt/server/ui/templates/v4_run.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,13 +252,13 @@ <h4>Parameters</h4>
<td><input id="test_min_value_filter" type="text" name="test_min_value_filter" value="{{ options.test_min_value_filter }}"/></td>
</tr>
<tr>
<td><label for="agg_func">Aggregation Function</label></td>
<td><label for="aggregation_function">Aggregation Function</label></td>
<td>
<select id="agg_func" name="aggregation_fn">
<option value="min" {{ ('selected="selected"' if "min" == options.aggregation_fn else '')|safe}}>
Minimum</option>
<option value="median" {{ ('selected="selected"' if "median" == options.aggregation_fn else '')|safe}}>
Median</option>
<select id="aggregation_function" name="aggregation_function">
<option value="min" {{ 'selected="selected"' if options.aggregation_function == 'min' else '' }}>Minimum</option>
<option value="max" {{ 'selected="selected"' if options.aggregation_function == 'max' else '' }}>Maximum</option>
<option value="mean" {{ 'selected="selected"' if options.aggregation_function == 'mean' else '' }}>Mean</option>
<option value="median" {{ 'selected="selected"' if options.aggregation_function == 'median' else '' }}>Median</option>
</select>
</td>
</tr>
Expand Down
73 changes: 39 additions & 34 deletions lnt/server/ui/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,25 @@ def ts_data(ts):
}


def determine_aggregation_function(function_name):
"""
Return the aggregation function associated to the provided function name, or None if
the function name is unsupported.

This is used by dropdown menus that allow selecting from multiple aggregation functions.
"""
if function_name == 'min':
return lnt.util.stats.safe_min
elif function_name == 'max':
return lnt.util.stats.safe_max
elif function_name == 'mean':
return lnt.util.stats.mean
elif function_name == 'median':
return lnt.util.stats.median
else:
return None


@db_route('/submitRun', methods=('GET', 'POST'))
def submit_run():
"""Compatibility url that hardcodes testsuite to 'nts'"""
Expand Down Expand Up @@ -353,10 +372,10 @@ def __init__(self, run_id):
abort(404, "Invalid run id {}".format(run_id))

# Get the aggregation function to use.
aggregation_fn_name = request.args.get('aggregation_fn')
self.aggregation_fn = {'min': lnt.util.stats.safe_min,
'median': lnt.util.stats.median}.get(
aggregation_fn_name, lnt.util.stats.safe_min)
fn_name = request.args.get('aggregation_function', 'min')
aggregation_fn = determine_aggregation_function(fn_name)
if aggregation_fn is None:
abort(404, "Invalid aggregation function name {}".format(fn_name))

# Get the MW confidence level.
try:
Expand Down Expand Up @@ -428,7 +447,7 @@ def __init__(self, run_id):
session, self.run, baseurl=db_url_for('.index', _external=False),
result=None, compare_to=compare_to, baseline=baseline,
num_comparison_runs=self.num_comparison_runs,
aggregation_fn=self.aggregation_fn, confidence_lv=confidence_lv,
aggregation_fn=aggregation_fn, confidence_lv=confidence_lv,
styles=styles, classes=classes)
self.sri = self.data['sri']
note = self.data['visible_note']
Expand Down Expand Up @@ -516,7 +535,7 @@ def v4_run(id):
else:
test_min_value_filter = 0.0

options['aggregation_fn'] = request.args.get('aggregation_fn', 'min')
options['aggregation_function'] = request.args.get('aggregation_function', 'min')

# Get the test names.
test_info = session.query(ts.Test.name, ts.Test.id).\
Expand Down Expand Up @@ -961,30 +980,13 @@ def v4_tableau():

@v4_route("/graph")
def v4_graph():

session = request.session
ts = request.get_testsuite()
switch_min_mean_local = False

if 'switch_min_mean_session' not in flask.session:
flask.session['switch_min_mean_session'] = False
# Parse the view options.
options = {'min_mean_checkbox': 'min()'}
if 'submit' in request.args: # user pressed a button
if 'switch_min_mean' in request.args: # user checked mean() checkbox
flask.session['switch_min_mean_session'] = \
options['switch_min_mean'] = \
bool(request.args.get('switch_min_mean'))
switch_min_mean_local = flask.session['switch_min_mean_session']
else: # mean() check box is not checked
flask.session['switch_min_mean_session'] = \
options['switch_min_mean'] = \
bool(request.args.get('switch_min_mean'))
switch_min_mean_local = flask.session['switch_min_mean_session']
else: # new page was loaded by clicking link, not submit button
options['switch_min_mean'] = switch_min_mean_local = \
flask.session['switch_min_mean_session']

options = {}
options['aggregation_function'] = \
request.args.get('aggregation_function') # default determined later based on the field being graphed
options['hide_lineplot'] = bool(request.args.get('hide_lineplot'))
show_lineplot = not options['hide_lineplot']
options['show_mad'] = show_mad = bool(request.args.get('show_mad'))
Expand Down Expand Up @@ -1198,15 +1200,18 @@ def trace_name(name, test_name, field_name):

is_multisample = (len(values) > 1)

aggregation_fn = min
if switch_min_mean_local:
aggregation_fn = lnt.util.stats.agg_mean
if field.bigger_is_better:
aggregation_fn = max
fn_name = options.get('aggregation_function') or ('max' if field.bigger_is_better else 'min')
aggregation_fn = determine_aggregation_function(fn_name)
if aggregation_fn is None:
abort(404, "Invalid aggregation function name {}".format(fn_name))
agg_value = aggregation_fn(values)

# When aggregating multiple samples, it becomes unclear which sample to use for
# associated data like the run date, the order, etc. Use the index of the closest
# value in all the samples.
closest_value = sorted(values, key=lambda val: abs(val - agg_value))[0]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change I called out about reimplementing how we select related data associated to the aggregated point. Previously, for min we'd use the actual data point that we extracted the minimum value from. For mean we would always pick the first point's data (see how agg_mean returned 0 for the index).

Now, we select the point closest to the value we selected. I can't really imagine a case where this would give us an incorrect answer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't all samples have the same run and so the same order? Or do we end up aggregating samples from different runs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% certain I follow. Are you suggesting that we could select any index at random and it wouldn't matter (assuming all samples are from the same run)?

So far, I have been operating under the assumption that you could have different runs that provide samples for the same test and the same order. For example, you could run benchmark foo today on commit 123abc and submit a run for that:

run order test execution_time
1 123abc foo 10

Then, a few days later you decide to do more runs for the same test and the same commit since you find that point to be a bit noisy, and want to take the median-of-3 instead. You can do that by submitting your results with lnt import /path/to/db report.json --merge append. Note that lnt import defaults to --merge replace, which will replace the data for the existing run for the same order, which IMO is a bit strange but that's how it works. Your database now looks like this:

run order test execution_time
1 123abc foo 10
... ... ... ...
45 123abc foo 11
... ... ... ...
78 123abc foo 8

The samples you have for foo at commit 123abc all come from different runs, and the question is what run information you'll pick to display on the graph. With my patch:

  • if you aggregate using median (which is 10), we'd end up selecting run information for run 1, which is correct
  • if you aggregate using mean (which is 9.67), we'd end up selecting run information for run 1 since that's the closest data point to 9.67. I feel there's not a perfect choice
  • if you aggregate using min, we'll pick run 78, which is correct
  • if you aggregate using max, we'll pick run 45, which is correct
  • if you hypothetically had another sample of value 10 and were aggregating with median (which would be 10 == median(8, 10, 10, 11)), we'd pick the run information for either of these two 10 samples. That seems acceptable.

I just tested this with synthetic data and it seems to behave the way I described above. Let me know if that makes sense to you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge assuming you're happy with this explanation. If not, let's talk and I can address that in another PR.

Copy link
Contributor

@lukel97 lukel97 Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting, your explanation makes sense. The way that we use aggregation on cc-perf.igalia.com is that we use --exec-multisample=3 to get multiple samples in a single run.

But I didn't consider that you might end up with multiple samples over multiple runs if you use --merge append. No problems with the fix here, thanks for talking it through.

agg_index = values.index(closest_value)

agg_value, agg_index = \
aggregation_fn((value, index)
for (index, value) in enumerate(values))
pts_y.append(agg_value)

# Plotly does not sort X axis in case of type: 'category'.
Expand Down
18 changes: 0 additions & 18 deletions lnt/util/stats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,6 @@ def geometric_mean(values):
return reduce(lambda a, b: a * b, [v ** iPow for v in values])


def agg_mean(pairs):
"""Aggregation function in views.py receives input via enumerate and
produces a tuple.
Input: (value, index)
Output: (mean, 0), or (None, None) on invalid input.
"""
if not pairs:
return None, None
my_sum = 0.0
counter = 0
for item in pairs:
my_sum += item[0]
counter += 1
if counter > 0:
return my_sum / counter, 0
return None, None


def median(values):
if not values:
return None
Expand Down
9 changes: 5 additions & 4 deletions tests/server/ui/V4Pages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,10 +636,11 @@ def main():
assert 2 == lines_in_function

# Make sure the new option does not break anything
check_html(client, '/db_default/v4/nts/graph?switch_min_mean=yes&plot.0=1.3.2&submit=Update')
check_json(client, '/db_default/v4/nts/graph?switch_min_mean=yes&plot.0=1.3.2&json=true&submit=Update')
check_html(client, '/db_default/v4/nts/graph?switch_min_mean=yes&plot.0=1.3.2')
check_json(client, '/db_default/v4/nts/graph?switch_min_mean=yes&plot.0=1.3.2&json=true')
check_html(client, '/db_default/v4/nts/graph?aggregation_function=mean&plot.0=1.3.2&submit=Update')
check_json(client, '/db_default/v4/nts/graph?aggregation_function=mean&plot.0=1.3.2&json=true&submit=Update')
check_html(client, '/db_default/v4/nts/graph?aggregation_function=mean&plot.0=1.3.2')
check_json(client, '/db_default/v4/nts/graph?aggregation_function=mean&plot.0=1.3.2&json=true')
check_html(client, '/db_default/v4/nts/graph?aggregation_function=nonexistent&plot.0=1.3.2', expected_code=404)
app.testing = False
error_page = check_html(client, '/explode', expected_code=500)
assert re.search("division (or modulo )?by zero",
Expand Down
64 changes: 27 additions & 37 deletions tests/server/ui/statsTester.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,34 @@

import lnt.util.stats as stats

INDEX = 0


class TestLNTStatsTester(unittest.TestCase):

@staticmethod
def _loc_test_agg_mean(values):
if values is None:
return stats.agg_mean(None)
agg_value, agg_index = stats.agg_mean(
(value, index) for (index, value) in enumerate(values))
return agg_value, agg_index

def test_agg_mean(self):
test_list1 = [1, 2, 3, 4, 6]
self.assertEqual(TestLNTStatsTester._loc_test_agg_mean(test_list1),
(3.2, INDEX))
test_list2 = [1.0, 2.0, 3.0, 4.0]
self.assertEqual(TestLNTStatsTester._loc_test_agg_mean(test_list2),
(2.5, INDEX))
test_list3 = [1.0]
self.assertEqual(TestLNTStatsTester._loc_test_agg_mean(test_list3),
(1.0, INDEX))
self.assertEqual(TestLNTStatsTester._loc_test_agg_mean([]),
(None, None))
self.assertEqual(TestLNTStatsTester._loc_test_agg_mean(None),
(None, None))

# Test it exactly how it is called in views.py without indirection
agg_value, agg_index = stats.agg_mean(
(value, index) for (index, value) in enumerate(test_list1))
self.assertEqual((3.2, INDEX), (agg_value, agg_index))
agg_value, agg_index = stats.agg_mean(
(value, index) for (index, value) in enumerate(test_list2))
self.assertEqual((2.5, INDEX), (agg_value, agg_index))
agg_value, agg_index = stats.agg_mean(
(value, index) for (index, value) in enumerate(test_list3))
self.assertEqual((1.0, INDEX), (agg_value, agg_index))
def test_safe_min(self):
self.assertEqual(stats.safe_min([]), None)
self.assertEqual(stats.safe_min([1]), 1)
self.assertEqual(stats.safe_min([1, 2, 3]), 1)
self.assertEqual(stats.safe_min([3, 2, 1]), 1)
self.assertEqual(stats.safe_min([1, 1, 1]), 1)

def test_safe_max(self):
self.assertEqual(stats.safe_max([]), None)
self.assertEqual(stats.safe_max([1]), 1)
self.assertEqual(stats.safe_max([1, 2, 3]), 3)
self.assertEqual(stats.safe_max([3, 2, 1]), 3)
self.assertEqual(stats.safe_max([1, 1, 1]), 1)

def test_mean(self):
self.assertEqual(stats.mean([]), None)
self.assertEqual(stats.mean([1]), 1)
self.assertEqual(stats.mean([1, 2, 3]), 2)
self.assertEqual(stats.mean([3, 2, 1]), 2)
self.assertEqual(stats.mean([1, 1, 1]), 1)

def test_median(self):
self.assertEqual(stats.median([]), None)
self.assertEqual(stats.median([1]), 1)
self.assertEqual(stats.median([1, 2, 3]), 2)
self.assertEqual(stats.median([3, 2, 1]), 2)
self.assertEqual(stats.median([1, 1, 1]), 1)


if __name__ == '__main__':
Expand Down
Loading