Repository: conbench/conbench (language: python)

Issue:
benchmark result: rename `iterations` to `repetitions`
Micro benchmark frameworks often perform `I` iterations for a given thing and measure the total time by comparing start time and end time of that entire operation (that is, there is no further fine-grained timing insights for anything happening between those two points in time).

For example, Google Benchmark calls this concept "iterations": https://github.com/google/benchmark/blob/main/docs/user_guide.md#output-formats

From Conbench's point of view, we encourage users to submit 'multi-sample results', or maybe we should call them 'multi-repetition results'. Via the API and UI we however expose this concept as "iterations". Our "iterations" is not the same as their "iterations".

It would be a great communication/intuitiveness win to normalize our "iterations" to "repetitions". There is a clear hierarchy: one repetition might be comprised of many iterations.

Quoting myself from https://github.com/conbench/conbench/issues/579:

> I would then choose L small enough for t/L to still be robust, but allowing for doing N (e.g. 6) repetitions within a reasonable time frame, leading to N duration measurements.

@alistaire47 on https://github.com/conbench/conbench/issues/579#issuecomment-1466470552:

> And yeah, we should have some consistent opinion on how to translate repetitions vs iterations in this context; vocabulary is causing confusion.





Discussion / hints:
This is about changing the JSON API, replacing the key name "iterations" with "repetitions", and adjusting docs/spec/client tooling to it. 

Fix patch:
diff --git a/conbench/entities/benchmark_result.py b/conbench/entities/benchmark_result.py
index c575ce203..08b37816a 100644
--- a/conbench/entities/benchmark_result.py
+++ b/conbench/entities/benchmark_result.py
@@ -20,6 +20,7 @@
 import conbench.util
 from conbench.dbsession import current_session
 from conbench.numstr import numstr, numstr_dyn
+from conbench.units import KNOWN_UNIT_SYMBOLS_STR
 
 from ..entities._entity import (
     Base,
@@ -969,7 +970,7 @@ class BenchmarkResultStatsSchema(marshmallow.Schema):
                 systematic stability analysis.
 
                 Values are expected to be ordered in the order the
-                iterations/repetitions were executed (the first element
+                repetitions were executed (the first element
                 corresponds to the first repetition, the second element is the
                 second repetition, etc.).
 
@@ -1016,9 +1017,14 @@ class BenchmarkResultStatsSchema(marshmallow.Schema):
         # I think in marshmallow terms this unfortunately means that empty
         # strings are/were allowed to be injected. TODO: be more strict.
         # Require users to provide a unit (non-zero length string)./
+        # Update: validation for units is now done strictly for non-errored
+        # results.
         required=True,
         metadata={
-            "description": "The unit of the measurement result (object (e.g. seconds, B/s)"
+            "description": (
+                "Unit of the numbers in `data`. Allowed values: "
+                + KNOWN_UNIT_SYMBOLS_STR
+            )
         },
     )
     time_unit = marshmallow.fields.String(
@@ -1031,9 +1037,17 @@ class BenchmarkResultStatsSchema(marshmallow.Schema):
         },
     )
     iterations = marshmallow.fields.Integer(
+        # TODO: make this not required, and clarify that these are
+        # microbenchmark iterations, stored as metadata.
+        # https://github.com/conbench/conbench/issues/1398
         required=True,
         metadata={
-            "description": "Number of iterations that were executed (should be the length of `data` and `times`)"
+            "description": (
+                "Here you can optionally store the number of microbenchmark "
+                "iterations executed (per repetition). Treated as metadata. "
+                "Do not store the number of repetitions here; this is reflected "
+                "by the length of the `data` array."
+            )
         },
     )
     min = marshmallow.fields.Decimal(


Test patch:
diff --git a/conbench/tests/api/_expected_docs.py b/conbench/tests/api/_expected_docs.py
index 65d66df7e..9bc6b55ce 100644
--- a/conbench/tests/api/_expected_docs.py
+++ b/conbench/tests/api/_expected_docs.py
@@ -1115,7 +1115,7 @@
             "BenchmarkResultStats": {
                 "properties": {
                     "data": {
-                        "description": 'A list of measurement results (e.g. duration, throughput).  Each value in this list is meant to correspond to one repetition of ideally the exact same measurement.  We recommend to repeat a measurement N times (3-6) for enabling systematic stability analysis.  Values are expected to be ordered in the order the iterations/repetitions were executed (the first element corresponds to the first repetition, the second element is the second repetition, etc.).  Values must be numeric or `null`: if one repetition failed but others did not you can mark the failed repetition as `null`.  Note that you may populate both this field and the "error" field in the top level of the benchmark result payload.  If any of the values in `data` is `null` or if the `error` field is set then Conbench will not include any of the reported data in automated analyses.',
+                        "description": 'A list of measurement results (e.g. duration, throughput).  Each value in this list is meant to correspond to one repetition of ideally the exact same measurement.  We recommend to repeat a measurement N times (3-6) for enabling systematic stability analysis.  Values are expected to be ordered in the order the repetitions were executed (the first element corresponds to the first repetition, the second element is the second repetition, etc.).  Values must be numeric or `null`: if one repetition failed but others did not you can mark the failed repetition as `null`.  Note that you may populate both this field and the "error" field in the top level of the benchmark result payload.  If any of the values in `data` is `null` or if the `error` field is set then Conbench will not include any of the reported data in automated analyses.',
                         "items": {"nullable": True, "type": "number"},
                         "type": "array",
                     },
@@ -1124,7 +1124,7 @@
                         "type": "number",
                     },
                     "iterations": {
-                        "description": "Number of iterations that were executed (should be the length of `data` and `times`)",
+                        "description": "Here you can optionally store the number of microbenchmark iterations executed (per repetition). Treated as metadata. Do not store the number of repetitions here; this is reflected by the length of the `data` array.",
                         "type": "integer",
                     },
                     "max": {
@@ -1165,7 +1165,7 @@
                         "type": "array",
                     },
                     "unit": {
-                        "description": "The unit of the measurement result (object (e.g. seconds, B/s)",
+                        "description": "Unit of the numbers in `data`. Allowed values: B/s, s, ns, i/s",
                         "type": "string",
                     },
                 },