Simplify call structure when updating Stats member variables. (stanford-crfm#1927)

Author: brianwgoldman

src/helm/benchmark/metrics/statistic.py

@@ -17,7 +17,18 @@ class Stat:
     max: Optional[float] = None
     mean: Optional[float] = None
     variance: Optional[float] = None
+    """This is the population variance, not the sample variance.
+
+    See https://towardsdatascience.com/variance-sample-vs-population-3ddbd29e498a
+    for details.
+    """
+
     stddev: Optional[float] = None
+    """This is the population standard deviation, not the sample standard deviation.
+
+    See https://towardsdatascience.com/variance-sample-vs-population-3ddbd29e498a
+    for details.
+    """
 
     def add(self, x) -> "Stat":
         # Skip Nones for statistic aggregation.
@@ -69,22 +80,17 @@ def process(x: Optional[float]) -> str:
         else:
             return "(0)"
 
-    def _update_mean(self):
-        self.mean = self.sum / self.count if self.count else None
-
-    def _update_variance(self):
-        self._update_mean()
-        if self.mean is None:
-            return None
+    def _update_mean_variance_stddev(self):
+        if self.count == 0:
+            # No stats with no elements.
+            return
+        # Update mean
+        self.mean = self.sum / self.count
+        # Update variance
         pvariance = self.sum_squared / self.count - self.mean**2
         self.variance = 0 if pvariance < 0 else pvariance
-
-    def _update_stddev(self):
-        self._update_variance()
-        self.stddev = math.sqrt(self.variance) if self.variance is not None else None
-
-    def _update_mean_variance_stddev(self):
-        self._update_stddev()
+        # Update stddev
+        self.stddev = math.sqrt(self.variance)
 
     def take_mean(self):
         """Return a version of the stat that only has the mean."""

src/helm/benchmark/metrics/test_statistic.py

@@ -1,9 +1,26 @@
 from typing import Dict
 
+import pytest
+import statistics
+
 from .metric_name import MetricName
 from .statistic import Stat, merge_stat
 
 
+def test_stat_add():
+    stat = Stat(MetricName("some_metric"))
+    population = list(range(10))
+    for i in population:
+        stat.add(i)
+    assert stat.sum == sum(population)
+    assert stat.count == 10
+    assert stat.min == 0
+    assert stat.max == 9
+    assert stat.mean == sum(population) / 10
+    assert stat.variance == pytest.approx(statistics.pvariance(population))
+    assert stat.stddev == pytest.approx(statistics.pstdev(population))
+
+
 def test_merge_stat():
     # Ensure that `MetricName`s are hashable
     metric_name = MetricName("some_metric")
@@ -12,3 +29,15 @@ def test_merge_stat():
 
     assert len(stats) == 1
     assert stats[metric_name].sum == 2
+    assert stats[metric_name].mean == 1
+
+
+def test_merge_empty_stat():
+    # This test ensures we guard against division by zero.
+    metric_name = MetricName("some_metric")
+    empty_1 = Stat(metric_name)
+    empty_2 = Stat(metric_name)
+    merged = empty_1.merge(empty_2)
+
+    assert merged.count == 0
+    assert merged.stddev is None