Skip to content

Commit cee2b5a

Browse files
jbrockmendelclaude
andcommitted
BUG: ewm(adjust=False) wrong result with com=1 and NaN (GH#31178)
pandas-devGH-59142 added a `com == 1` special case to the ewm loop to handle irregular intervals when `times` is provided, but did not gate it on `times` actually being given, so it leaked into the equally-spaced path. With `adjust=False`, `ignore_na=False` and NaN present, com=1 (equivalently span=3, alpha=0.5, halflife=1) then returned a value discontinuous in alpha: `Series([1, NaN, 5]).ewm(com=1, adjust=False).mean()` gave 4.0 rather than 3.6667, while com=1±eps gave the correct result. Gate the branch on whether `times` was provided. The cython loop already derives this as `deltas is not None`; both numba generators always receive `deltas`, so thread a `use_deltas` flag through from `ewm.py`. The `com == 1` term is dropped: when `times` is provided with `adjust=False`, `ExponentialMovingWindow.__init__` rejects com/span/alpha and forces `_com = 1.0`, so the test was invariantly true under the new gate and only served to encode the com-dependence that caused this bug. Also document the `adjust=False` + `ignore_na` recursion in window.rst, which was the original 2020 question in the same issue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 7986b42 commit cee2b5a

7 files changed

Lines changed: 89 additions & 8 deletions

File tree

doc/source/user_guide/window.rst

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,30 @@ Whereas if ``ignore_na=True``, the weighted average would be calculated as
612612
613613
\frac{(1-\alpha) \cdot 3 + 1 \cdot 5}{(1-\alpha) + 1}.
614614
615+
``ignore_na`` makes the same distinction when ``adjust=False``, except that the
616+
decay is applied to the running average rather than to each observation. For
617+
equally spaced points (that is, when ``times`` is not supplied), letting
618+
:math:`u` be the index of the most recent non-null observation before :math:`t`,
619+
the recursion with ``ignore_na=False`` is
620+
621+
.. math::
622+
623+
y_t = \frac{(1-\alpha)^{t-u} y_u + \alpha x_t}{(1-\alpha)^{t-u} + \alpha},
624+
625+
so the weighted average of ``3, NaN, 5`` is
626+
627+
.. math::
628+
629+
\frac{(1-\alpha)^2 \cdot 3 + \alpha \cdot 5}{(1-\alpha)^2 + \alpha}.
630+
631+
With ``ignore_na=True`` the intermediate null does not count towards the
632+
exponent, so it is always :math:`1` and the recursion reduces to
633+
:math:`y_t = (1-\alpha) y_u + \alpha x_t`, giving
634+
635+
.. math::
636+
637+
(1-\alpha) \cdot 3 + \alpha \cdot 5.
638+
615639
The :meth:`~ExponentialMovingWindow.var`, :meth:`~ExponentialMovingWindow.std`, and :meth:`~ExponentialMovingWindow.cov` functions have a ``bias`` argument,
616640
specifying whether the result should contain biased or unbiased statistics.
617641
For example, if ``bias=True``, ``ewmvar(x)`` is calculated as

doc/source/whatsnew/v3.1.0.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,7 @@ Groupby/resample/rolling
580580
- Bug in :meth:`.Rolling.skew` and :meth:`.Rolling.kurt` returning ``NaN`` for low-variance windows (:issue:`62946`)
581581
- Bug in :meth:`.Rolling.sum`, :meth:`.Rolling.mean`, :meth:`.Rolling.median`, :meth:`.Rolling.min`, and :meth:`.Rolling.max` with ``method="table"``, ``engine="numba"``, and ``engine_kwargs={"parallel": True}`` could cause a segfault (:issue:`40454`)
582582
- Bug in :meth:`.SeriesGroupBy.ohlc` ignoring ``as_index=False`` (:issue:`65140`)
583+
- Bug in :meth:`DataFrame.ewm` and :meth:`Series.ewm` with ``adjust=False`` and ``ignore_na=False`` returning incorrect results for data containing ``NaN`` when the decay was given as ``com=1`` (equivalently ``span=3``, ``alpha=0.5``, or ``halflife=1``), a regression in 3.0.0; the result no longer differs from that of a decay an arbitrarily small amount away (:issue:`31178`)
583584
- Bug in :meth:`DataFrame.groupby` with a :class:`Grouper` with ``freq`` raising ``AttributeError`` when all grouping keys are ``NaT`` (:issue:`43486`)
584585
- Bug in :meth:`DataFrame.resample` and :meth:`Series.resample` with a timezone-naive index where using a ``Day`` frequency (e.g. ``"7D"``) produced different bin edges than the equivalent ``Hour`` frequency (e.g. ``"168h"``), and where ``origin`` and ``offset`` were ignored (:issue:`44996`, :issue:`62200`)
585586
- Bug in :meth:`DataFrame.resample` dropping the result index name when resampling ``on`` a column with a pyarrow-backed datetime or duration dtype (:issue:`59823`)

pandas/_libs/window/aggregations.pyx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,8 +2125,10 @@ def ewm(const float64_t[:] vals, const int64_t[:] start, const int64_t[:] end,
21252125
if normalize:
21262126
# avoid numerical errors on constant series
21272127
if weighted != cur:
2128-
if not adjust and com == 1:
2129-
# update in case of irregular-interval series
2128+
if use_deltas and not adjust:
2129+
# times were provided: weight by the
2130+
# elapsed interval so old_wt and new_wt
2131+
# sum to 1 (GH#31178)
21302132
new_wt = 1. - old_wt
21312133
weighted = old_wt * weighted + new_wt * cur
21322134
weighted /= (old_wt + new_wt)

pandas/core/window/ewm.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ def mean(
602602
adjust=self.adjust,
603603
ignore_na=self.ignore_na,
604604
deltas=tuple(self._deltas),
605+
use_deltas=self.times is not None,
605606
normalize=True,
606607
)
607608
return self._apply(ewm_func, name="mean")
@@ -693,6 +694,7 @@ def sum(
693694
adjust=self.adjust,
694695
ignore_na=self.ignore_na,
695696
deltas=tuple(self._deltas),
697+
use_deltas=self.times is not None,
696698
normalize=False,
697699
)
698700
return self._apply(ewm_func, name="sum")

pandas/core/window/numba_.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def generate_numba_ewm_func(
8383
adjust: bool,
8484
ignore_na: bool,
8585
deltas: tuple,
86+
use_deltas: bool,
8687
normalize: bool,
8788
):
8889
"""
@@ -99,6 +100,9 @@ def generate_numba_ewm_func(
99100
adjust : bool
100101
ignore_na : bool
101102
deltas : tuple
103+
use_deltas : bool
104+
Whether `deltas` reflects user-provided `times`. False means the points
105+
are equally spaced and `deltas` is all ones.
102106
normalize : bool
103107
104108
Returns
@@ -143,8 +147,9 @@ def ewm(
143147
# note that len(deltas) = len(vals) - 1 and deltas[i]
144148
# is to be used in conjunction with vals[i+1]
145149
old_wt *= old_wt_factor ** deltas[start + j - 1]
146-
if not adjust and com == 1:
147-
# update in case of irregular-interval time series
150+
if use_deltas and not adjust:
151+
# times were provided: weight by the elapsed interval
152+
# so old_wt and new_wt sum to 1 (GH#31178)
148153
new_wt = 1.0 - old_wt
149154
else:
150155
weighted = old_wt_factor * weighted
@@ -265,6 +270,7 @@ def generate_numba_ewm_table_func(
265270
adjust: bool,
266271
ignore_na: bool,
267272
deltas: tuple,
273+
use_deltas: bool,
268274
normalize: bool,
269275
):
270276
"""
@@ -281,6 +287,9 @@ def generate_numba_ewm_table_func(
281287
adjust : bool
282288
ignore_na : bool
283289
deltas : tuple
290+
use_deltas : bool
291+
Whether `deltas` reflects user-provided `times`. False means the points
292+
are equally spaced and `deltas` is all ones.
284293
normalize: bool
285294
286295
Returns
@@ -319,8 +328,9 @@ def ewm_table(
319328
# note that len(deltas) = len(vals) - 1 and deltas[i]
320329
# is to be used in conjunction with vals[i+1]
321330
old_wt[j] *= old_wt_factor ** deltas[i - 1]
322-
if not adjust and com == 1:
323-
# update in case of irregular-interval time series
331+
if use_deltas and not adjust:
332+
# times were provided: weight by the elapsed interval
333+
# so old_wt and new_wt sum to 1 (GH#31178)
324334
new_wt = 1.0 - old_wt[j]
325335
else:
326336
weighted[j] = old_wt_factor * weighted[j]

pandas/tests/window/test_ewm.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ def test_ewma_halflife_without_times(halflife_with_times):
9393
)
9494
@pytest.mark.parametrize("min_periods", [0, 2])
9595
def test_ewma_with_times_equal_spacing(halflife_with_times, times, min_periods):
96+
# Deliberately not parametrized over adjust: with adjust=False and
97+
# ignore_na=False the times path weights NaN gaps as a convex combination
98+
# over the elapsed interval, which differs from the no-times recursion
99+
# (GH#31178)
96100
halflife = halflife_with_times
97101
data = np.arange(10.0)
98102
data[::2] = np.nan
@@ -437,6 +441,29 @@ def test_ewma_nan_handling_cases(s, adjust, ignore_na, w):
437441
tm.assert_series_equal(result, expected)
438442

439443

444+
@pytest.mark.parametrize(
445+
"kwargs", [{"com": 1}, {"span": 3}, {"alpha": 0.5}, {"halflife": 1}]
446+
)
447+
def test_ewma_nan_handling_adjust_false_com1(kwargs):
448+
# GH#31178 com=1 was special-cased for the times= path, but the branch was
449+
# not gated on times being provided, so it leaked into the equally-spaced
450+
# path and returned 4.0 here.
451+
ser = Series([1.0, np.nan, 5.0])
452+
result = ser.ewm(adjust=False, ignore_na=False, **kwargs).mean()
453+
# alpha = 0.5, so y[2] = (0.5**2 * 1 + 0.5 * 5) / (0.5**2 + 0.5)
454+
expected = Series([1.0, 1.0, 11 / 3])
455+
tm.assert_series_equal(result, expected)
456+
457+
458+
def test_ewma_nan_handling_adjust_false_com1_continuous():
459+
# GH#31178 com=1 must not be discontinuous in alpha
460+
ser = Series([1.0, np.nan, 5.0])
461+
result = ser.ewm(com=1, adjust=False, ignore_na=False).mean()
462+
for com in [1 - 1e-9, 1 + 1e-9]:
463+
neighbor = ser.ewm(com=com, adjust=False, ignore_na=False).mean()
464+
tm.assert_series_equal(result, neighbor, atol=1e-8, rtol=0)
465+
466+
440467
def test_ewm_alpha():
441468
# GH 10789
442469
arr = np.random.default_rng(2).standard_normal(100)

pandas/tests/window/test_numba.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,8 +337,23 @@ def test_cython_vs_numba(self, grouper, method, nogil, parallel, ignore_na, adju
337337

338338
tm.assert_frame_equal(result, expected)
339339

340+
@pytest.mark.parametrize("method", ["single", "table"])
341+
def test_ewm_nan_adjust_false_com1(self, method, nogil, parallel):
342+
# GH#31178 com=1 was special-cased in both numba ewm generators without
343+
# gating on times being provided. Checked against the expected value
344+
# rather than the cython engine, which had the same bug.
345+
df = DataFrame({"B": [1.0, np.nan, 5.0]})
346+
ewm = df.ewm(com=1, adjust=False, ignore_na=False, method=method)
347+
348+
engine_kwargs = {"nogil": nogil, "parallel": parallel}
349+
result = ewm.mean(engine="numba", engine_kwargs=engine_kwargs)
350+
# alpha = 0.5, so y[2] = (0.5**2 * 1 + 0.5 * 5) / (0.5**2 + 0.5)
351+
expected = DataFrame({"B": [1.0, 1.0, 11 / 3]})
352+
353+
tm.assert_frame_equal(result, expected)
354+
340355
@pytest.mark.parametrize("grouper", ["None", "groupby"])
341-
def test_cython_vs_numba_times(self, grouper, nogil, parallel, ignore_na):
356+
def test_cython_vs_numba_times(self, grouper, nogil, parallel, ignore_na, adjust):
342357
# GH 40951
343358

344359
df = DataFrame({"B": [0, 0, 1, 1, 2, 2]})
@@ -360,7 +375,7 @@ def test_cython_vs_numba_times(self, grouper, nogil, parallel, ignore_na):
360375
]
361376
)
362377
ewm = grouper(df).ewm(
363-
halflife=halflife, adjust=True, ignore_na=ignore_na, times=times
378+
halflife=halflife, adjust=adjust, ignore_na=ignore_na, times=times
364379
)
365380

366381
engine_kwargs = {"nogil": nogil, "parallel": parallel}

0 commit comments

Comments
 (0)