Fix sign of Gaussian LL (#467)

Quantco · Oct 14, 2021 · 5a2f7c7 · 5a2f7c7
1 parent d15c42a
commit 5a2f7c7
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 5 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -7,6 +7,14 @@
 Changelog
 =========
 
+Unreleased
+----------
+
+**Bug fix:**
+
+- Fixed the sign of the log likelihood of the Gaussian distribution (not used for fitting coefficients).
+
+
 2.0.1 - 2021-10-11
 ------------------
 
@@ -19,7 +27,7 @@ Changelog
 
 **Breaking changes:**
 
-- Renamed the package to ``glum``!! Hurray! Celebration. 
+- Renamed the package to ``glum``!! Hurray! Celebration.
 - :class:`~glum.GeneralizedLinearRegressor` and :class:`~glum.GeneralizedLinearRegressorCV` lose the ``fit_dispersion`` parameter.
   Please use the :meth:`dispersion` method of the appropriate family instance instead.
 - All functions now use ``sample_weight`` as a keyword instead of ``weights``, in line with scikit-learn.

diff --git a/src/glum/_functions.pyx b/src/glum/_functions.pyx
@@ -80,8 +80,8 @@ def normal_log_likelihood(
     cdef floating ll = 0.0  # output
 
     for i in prange(n, nogil=True):
-        ll += weights[i] * (y[i] - mu[i]) ** 2
-        sum_weights += weights[i]
+        ll -= weights[i] * (y[i] - mu[i]) ** 2
+        sum_weights -= weights[i]
 
     return ll / (2 * dispersion) + sum_weights * log(2 * M_PI * dispersion) / 2
 

diff --git a/tests/glm/test_distribution.py b/tests/glm/test_distribution.py
@@ -327,7 +327,7 @@ def test_gaussian_deviance_dispersion_loglihood(family, weighted):
     # glm_model$coefficients  # 0.2
     # sum(glm_model$weights * glm_model$residuals^2)/4  # 1.7
     # glm_model$deviance  # 6.8
-    # logLik(glm_model)  # 7.863404 (df=2)
+    # logLik(glm_model)  # -7.863404 (df=2)
 
     regressor = GeneralizedLinearRegressor(
         alpha=0,
@@ -361,7 +361,7 @@ def test_gaussian_deviance_dispersion_loglihood(family, weighted):
     np.testing.assert_approx_equal(regressor.coef_[0], 0.2)
     np.testing.assert_approx_equal(family.dispersion(y, mu, sample_weight=wgts), 1.7)
     np.testing.assert_approx_equal(family.deviance(y, mu, sample_weight=wgts), 6.8)
-    np.testing.assert_approx_equal(ll, 7.863404)
+    np.testing.assert_approx_equal(ll, -7.863404)
 
 
 @pytest.mark.parametrize("weighted", [False, True])