API: negative percent changed is always a regression

conbench · May 27, 2021 · 1e38445 · 1e38445
1 parent 8be76aa
commit 1e38445
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 28 deletions.
diff --git a/conbench/app/compare.py b/conbench/app/compare.py
@@ -124,10 +124,6 @@ def _compare(self, params):
                 compare = f'{c["baseline_batch_id"]}...{c["contender_batch_id"]}'
                 c["compare_batches_url"] = f.url_for(view, compare_ids=compare)
 
-                c["change"] = float(c["change"][:-1])
-                if c["less_is_better"] and c["change"] != 0:
-                    c["change"] = c["change"] * -1
-
                 if c["regression"]:
                     regressions += 1
                 if c["improvement"]:

diff --git a/conbench/entities/_comparator.py b/conbench/entities/_comparator.py
@@ -107,19 +107,19 @@ def change(self):
         if old == 0:
             return 0.0
 
-        return (new - old) / abs(old)
+        result = (new - old) / abs(old)
+        if self.less_is_better and result != 0:
+            result = result * -1
+
+        return result
 
     @property
     def regression(self):
-        change = self.change
-        adjusted_change = change if self.less_is_better else -change
-        return adjusted_change * 100 > self.threshold
+        return -self.change * 100 > self.threshold
 
     @property
     def improvement(self):
-        change = self.change
-        adjusted_change = -change if self.less_is_better else change
-        return adjusted_change * 100 > self.threshold
+        return self.change * 100 > self.threshold
 
     @property
     def baseline_z_score(self):

diff --git a/conbench/templates/compare-entity.html b/conbench/templates/compare-entity.html
@@ -50,7 +50,7 @@
                          <td>{{ c.baseline }}</td>
                          <td>{{ c.contender }}</td>
                          <td>
-                           {{ c.change }}%
+                           {{ c.change }}
                            {% if c.regression %}
                            <span class="glyphicon glyphicon-arrow-down"></span></b>
                            {% endif %}

diff --git a/conbench/templates/compare-list.html b/conbench/templates/compare-list.html
@@ -72,7 +72,7 @@
                 <tbody>
                     {% for c in comparisons %}
                     <tr>
-                         <td>{{ c.change }}%</td>
+                         <td>{{ c.change }}</td>
                          <td>
                            {% if type == "batch" %}
                              <div>{{ c.batch }}</div>

diff --git a/conbench/tests/api/test_compare.py b/conbench/tests/api/test_compare.py
@@ -89,7 +89,7 @@ def test_compare(self, client):
             {
                 "baseline": "2.000 s",
                 "contender": "5.000 s",
-                "change": "150.000%",
+                "change": "-150.000%",
                 "regression": True,
                 "baseline_z_score": "-0.302",
                 "contender_z_score": "3.015",

diff --git a/conbench/tests/entities/test_comparator.py b/conbench/tests/entities/test_comparator.py
@@ -193,7 +193,7 @@ def test_compare_regression_less_is_better():
     assert result == {
         "batch": "arrow-compute-scalar-cast-benchmark",
         "benchmark": "CastUInt32ToInt32Safe/262144/1000",
-        "change": "6.000",
+        "change": "-6.000",
         "threshold": "5.000",
         "regression": True,
         "improvement": False,
@@ -219,7 +219,7 @@ def test_compare_regression_less_is_better():
     assert formatted == {
         "batch": "arrow-compute-scalar-cast-benchmark",
         "benchmark": "CastUInt32ToInt32Safe/262144/1000",
-        "change": "6.000%",
+        "change": "-6.000%",
         "threshold": "5.000%",
         "regression": True,
         "improvement": False,
@@ -520,7 +520,7 @@ def test_compare_improvement_less_is_better():
     assert result == {
         "batch": "arrow-compute-scalar-cast-benchmark",
         "benchmark": "CastUInt32ToInt32Safe/262144/1000",
-        "change": "-6.000",
+        "change": "6.000",
         "threshold": "5.000",
         "regression": False,
         "improvement": True,
@@ -546,7 +546,7 @@ def test_compare_improvement_less_is_better():
     assert formatted == {
         "batch": "arrow-compute-scalar-cast-benchmark",
         "benchmark": "CastUInt32ToInt32Safe/262144/1000",
-        "change": "-6.000%",
+        "change": "6.000%",
         "threshold": "5.000%",
         "regression": False,
         "improvement": True,
@@ -821,7 +821,7 @@ def test_compare_list():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491",
+            "change": "96.491",
             "threshold": "5.000",
             "regression": False,
             "improvement": True,
@@ -875,7 +875,7 @@ def test_compare_list():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491%",
+            "change": "96.491%",
             "threshold": "5.000%",
             "regression": False,
             "improvement": True,
@@ -975,7 +975,7 @@ def test_compare_list_missing_contender():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491",
+            "change": "96.491",
             "threshold": "5.000",
             "regression": False,
             "improvement": True,
@@ -1029,7 +1029,7 @@ def test_compare_list_missing_contender():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491%",
+            "change": "96.491%",
             "threshold": "5.000%",
             "regression": False,
             "improvement": True,
@@ -1130,7 +1130,7 @@ def test_compare_list_empty_contender():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491",
+            "change": "96.491",
             "threshold": "5.000",
             "regression": False,
             "improvement": True,
@@ -1184,7 +1184,7 @@ def test_compare_list_empty_contender():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491%",
+            "change": "96.491%",
             "threshold": "5.000%",
             "regression": False,
             "improvement": True,
@@ -1284,7 +1284,7 @@ def test_compare_list_missing_baseline():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491",
+            "change": "96.491",
             "threshold": "5.000",
             "regression": False,
             "improvement": True,
@@ -1338,7 +1338,7 @@ def test_compare_list_missing_baseline():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491%",
+            "change": "96.491%",
             "threshold": "5.000%",
             "regression": False,
             "improvement": True,
@@ -1439,7 +1439,7 @@ def test_compare_list_empty_baseline():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491",
+            "change": "96.491",
             "threshold": "5.000",
             "regression": False,
             "improvement": True,
@@ -1493,7 +1493,7 @@ def test_compare_list_empty_baseline():
         {
             "batch": "math",
             "benchmark": "subtraction",
-            "change": "-96.491%",
+            "change": "96.491%",
             "threshold": "5.000%",
             "regression": False,
             "improvement": True,