From 14d8fa24d7870cd28f419cd60639cfff65b456a3 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 2 Oct 2025 12:05:38 -0400 Subject: [PATCH 1/3] Make aggregation function selection consistent across Run and Chart pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, the Run page (e.g. /db_default/v4/nts/) provides a way to select the aggregation function used when there are multiple samples in the run (that option is located under "View Options"). The supported options are min and median. Similarly, the Graph page (e.g. /db_default/v4/nts/graph?) provides a way to use mean instead of min to aggregate multiple data points for the same order on the graph. That is provided as a checkbox under the "Settings 🔧" menu. This patch makes both selection mechanisms consistent by using a dropdown menu that provides min, max, mean and median in both cases. As a drive-by, it reimplements how we select the sample to use as a reference for related data: we now select the point closest to the aggregated value instead of selecting the first point unconditionally. --- lnt/server/ui/templates/v4_graph.html | 15 ++++-- lnt/server/ui/templates/v4_run.html | 12 ++--- lnt/server/ui/views.py | 66 +++++++++++++-------------- lnt/util/stats.py | 18 -------- tests/server/ui/V4Pages.py | 8 ++-- tests/server/ui/statsTester.py | 64 +++++++++++--------------- 6 files changed, 81 insertions(+), 102 deletions(-) diff --git a/lnt/server/ui/templates/v4_graph.html b/lnt/server/ui/templates/v4_graph.html index 82a784f4..79270bf1 100644 --- a/lnt/server/ui/templates/v4_graph.html +++ b/lnt/server/ui/templates/v4_graph.html @@ -137,9 +137,18 @@

Controls

{{ 'checked="checked"' if options.xaxis_date else ""}}/> - Mean() as Aggregation - + Aggregation function: + + {# Split this into a new row to avoid making the dialog wider. #} + + + + Hide Line Plot: diff --git a/lnt/server/ui/templates/v4_run.html b/lnt/server/ui/templates/v4_run.html index fc1d9d86..5ff439e6 100644 --- a/lnt/server/ui/templates/v4_run.html +++ b/lnt/server/ui/templates/v4_run.html @@ -252,13 +252,13 @@

Parameters

- + - + + + + diff --git a/lnt/server/ui/views.py b/lnt/server/ui/views.py index 91191e9b..5a39836a 100644 --- a/lnt/server/ui/views.py +++ b/lnt/server/ui/views.py @@ -186,6 +186,23 @@ def ts_data(ts): } +def determine_aggregation_function(function_name): + """ + Return the aggregation function associated to the provided function name. 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: + assert False, f'Invalid aggregation function name {function_name}' + + @db_route('/submitRun', methods=('GET', 'POST')) def submit_run(): """Compatibility url that hardcodes testsuite to 'nts'""" @@ -353,10 +370,7 @@ 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) + aggregation_fn = determine_aggregation_function(request.args.get('aggregation_function', 'min')) # Get the MW confidence level. try: @@ -428,7 +442,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'] @@ -516,7 +530,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).\ @@ -961,30 +975,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')) @@ -1198,15 +1195,16 @@ 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) + 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] + 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'. diff --git a/lnt/util/stats.py b/lnt/util/stats.py index e9e23ea1..383ef980 100644 --- a/lnt/util/stats.py +++ b/lnt/util/stats.py @@ -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 diff --git a/tests/server/ui/V4Pages.py b/tests/server/ui/V4Pages.py index 9abe1518..8656ab13 100644 --- a/tests/server/ui/V4Pages.py +++ b/tests/server/ui/V4Pages.py @@ -636,10 +636,10 @@ 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') app.testing = False error_page = check_html(client, '/explode', expected_code=500) assert re.search("division (or modulo )?by zero", diff --git a/tests/server/ui/statsTester.py b/tests/server/ui/statsTester.py index 30cd2a71..ab17a67b 100644 --- a/tests/server/ui/statsTester.py +++ b/tests/server/ui/statsTester.py @@ -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__': From 6dafde9b6c363fbddb712617ca73e3779d3d841f Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Thu, 2 Oct 2025 19:33:54 -0400 Subject: [PATCH 2/3] Address review comments for 404 instead of assert --- lnt/server/ui/views.py | 15 +++++++++++---- tests/server/ui/V4Pages.py | 1 + 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lnt/server/ui/views.py b/lnt/server/ui/views.py index 5a39836a..79604c1f 100644 --- a/lnt/server/ui/views.py +++ b/lnt/server/ui/views.py @@ -188,8 +188,10 @@ def ts_data(ts): def determine_aggregation_function(function_name): """ - Return the aggregation function associated to the provided function name. This is used by - dropdown menus that allow selecting from multiple aggregation functions. + 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 @@ -200,7 +202,7 @@ def determine_aggregation_function(function_name): elif function_name == 'median': return lnt.util.stats.median else: - assert False, f'Invalid aggregation function name {function_name}' + return None @db_route('/submitRun', methods=('GET', 'POST')) @@ -370,7 +372,10 @@ def __init__(self, run_id): abort(404, "Invalid run id {}".format(run_id)) # Get the aggregation function to use. - aggregation_fn = determine_aggregation_function(request.args.get('aggregation_function', '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: @@ -1197,6 +1202,8 @@ def trace_name(name, test_name, field_name): 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 diff --git a/tests/server/ui/V4Pages.py b/tests/server/ui/V4Pages.py index 8656ab13..385f388c 100644 --- a/tests/server/ui/V4Pages.py +++ b/tests/server/ui/V4Pages.py @@ -640,6 +640,7 @@ def main(): 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", From c72a973ab12b41df72b8088fb77d9b8a0d4f7fab Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Fri, 3 Oct 2025 16:34:31 -0400 Subject: [PATCH 3/3] Fix flake8 issues: GH action is working --- lnt/server/ui/views.py | 2 +- tests/server/ui/statsTester.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lnt/server/ui/views.py b/lnt/server/ui/views.py index 79604c1f..56687df1 100644 --- a/lnt/server/ui/views.py +++ b/lnt/server/ui/views.py @@ -986,7 +986,7 @@ def v4_graph(): # Parse the view options. options = {} options['aggregation_function'] = \ - request.args.get('aggregation_function') # default determined later based on the field being graphed + 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')) diff --git a/tests/server/ui/statsTester.py b/tests/server/ui/statsTester.py index ab17a67b..7ffc41fe 100644 --- a/tests/server/ui/statsTester.py +++ b/tests/server/ui/statsTester.py @@ -4,6 +4,7 @@ import lnt.util.stats as stats + class TestLNTStatsTester(unittest.TestCase): def test_safe_min(self): self.assertEqual(stats.safe_min([]), None)