Repository navigation
fix: a gap in the input never produces a silently wrong feature value - #32
Merged
Merged
Conversation
Three defects with one shape: an indicator handed a series with a gap returned a finite number that was simply wrong, with nothing raised and nothing null. amihud_illiquidity Amihud (2002) averages |R| / DVOL over the D periods the asset traded and divides by D. The implementation nulled untraded bars and took `rolling_mean(period)` over the result, which propagates a null across the whole following window, so it instead required every bar in the window to have traded. On daily equities the two agree because a listed name trades every day. On the NASDAQ-100 minute panel at period=30 they do not: 0.64% untraded bars produced a 9.08% null share, a 14x amplification. Now the denominator counts the bars in the window that traded. momentum.rsi Wilder's average is recursive and had no notion of a gap. Compiled with `fastmath=True`, which licenses LLVM to assume no NaN, one null in the input returned exactly 100.0 for 87.5% of the remaining series: median 49.55 -> 100.0 on a 4,000-point random walk with a single hole at index 500. The series is now split on NaN and each gap-free run is seeded on its own, so a gap costs the missing bar plus `period` warmup observations and the oscillator then resumes. Measured on the same series: the gap nulls indices 500-509 and the tail matches the clean series exactly. fastmath, across the feature kernels `fastmath=True` is what turned NaN propagation into silent garbage everywhere, not only in RSI. Feeding one null into every momentum, volatility and volume feature and comparing against the clean series, 20 of 91 feature-column pairs returned a finite value that differed from the correct one, up to 91.9 for mfi and 1.6e4 for the A/D line. Removing `fastmath` leaves 6; fixing obv, ad and the polars A/D path leaves 0. Every feature now either recovers from a gap or returns NaN. obv froze its level forever, because both comparisons against a NaN previous close are false. ad_polars used `cum_sum`, which skips nulls and carries the level on as if the bar had been observed, while ad_numba propagates and the docstring says it propagates. Both now propagate. Cost, 5M rows: rsi 42.7 -> 53.8 ms, atr 51.4 -> 90.3 ms, adx unchanged. No TA-Lib exactness test moved. tests/features/test_missing_observation_handling.py carries the property for all 18 recursive and cumulative features, and fails on 23 cases without these fixes. Note for anyone bisecting this: numba's `cache=True` does not invalidate when a jitted *callee* changes, so delete src/**/__pycache__/*.nb[ic] after editing a kernel or the old machine code is silently reused.
`polars.Expr.rolling_map` does not advance its window while the callback is being compiled. A feature whose callback calls a lazily-jitted Numba kernel therefore returned the first window's value for the whole series the first time a process evaluated it. Nothing raised and no value was NaN: the column simply came out near-constant, and every later evaluation in that process was correct. Measured on a 600-row random walk, distinct values returned on the first evaluation against the second, each in a fresh interpreter with an empty Numba cache: regime.hurst_exponent 2 -> 501 statistics.coefficient_of_variation 2 -> 576 statistics.rolling_cv_zscore 2 -> 551 statistics.rolling_drift 2 -> 576 ml.rolling_entropy_lz 2 -> 94 ml.rolling_entropy_plugin 2 -> 46 `warm_rolling_callback` runs the callback once on a probe window of the right length, so compilation happens before `rolling_map` starts. It is applied at all ten sites whose callback calls a kernel, including the four that measured clean, so no site depends on which kernels another feature happened to compile first. This is what made `etfs/03_financial_features` fail its withheld-rows check on CI and pass when re-run by hand. The notebook builds the panel, then rebuilds it with the holdout withheld, and compares value by value against a 1e-12 tolerance. The first build compiled and got a constant `hurst_100`; the second found the kernel compiled and got the real one, so the columns differed by up to 0.177 and the assertion named `hurst_100` as fitted across the sample. Reproducing by hand always warmed the cache first, which is why five separate reproductions returned a maximum difference of 0.0, and why it correlated with machine load: a loaded box was one building fresh worktrees and fresh caches. There is no leak. The withheld build reads no later rows. But the seal check was reporting something real, and a constant feature column in any fresh environment - CI, a new venv, a reader's first run - is worse than the failure that surfaced it. tests/features/test_rolling_map_first_call.py runs each of the twelve rolling_map-backed features in its own interpreter with its own NUMBA_CACHE_DIR and compares the first evaluation against the second. Six fail without this change.
…t carried them The RSI entry names the production evidence rather than the reproduction script: a crypto premium index with 537 nulls at 79.0% saturation, and a CME crude-oil series where one null on 2020-04-20 - the day the contract settled negative - saturated 12.6% of the product's rsi_14 for the following five years.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three defects with one shape: an indicator handed a series with a gap, or evaluated
for the first time in a process, returned a finite number that was simply wrong,
with nothing raised and nothing null.
Closes #30. Closes #31.
amihud_illiquidityaveraged over the whole window, not the traded bars (#31)Amihud (2002) averages
|R| / DVOLover the D periods the asset traded and dividesby D. Nulling untraded bars and taking
rolling_mean(period)propagates the nullacross the following window instead, so the implementation required every bar in
the window to have traded. On the NASDAQ-100 minute panel at
period=30, 0.64%untraded bars produced a 9.08% null share. The denominator now counts the bars in
the window that traded.
momentum.rsisaturated at 100 after one null (#30)Wilder's average is recursive and had no notion of a gap. Compiled with
fastmath=True, which licenses LLVM to assume no NaN, one null returned exactly100.0for 87.5% of the remaining series: median 49.55 -> 100.0 on a 4,000-pointrandom walk. The series is now split on NaN and each gap-free run is seeded on its
own, so a gap costs the missing bar plus
periodwarmup observations and theoscillator resumes. On that series the gap nulls indices 500-509 and the tail
matches the clean series exactly.
fastmathwas doing this across the feature surfacefastmath=Trueis what turned NaN propagation into silent garbage generally.Feeding one null into every momentum, volatility and volume feature and comparing
against the clean series, 20 of 91 feature-column pairs returned a finite value
that differed from the correct one, up to 91.9 for
mfiand 1.6e4 for the A/Dline. Removing
fastmathleaves 6; fixingobv,adand the polars A/D pathleaves 0. Every feature now either recovers from a gap or returns NaN.
obvfroze its level forever, because both comparisons against a NaN previousclose are false.
ad_polarsusedcum_sum, which skips nulls and carries the levelon as if the bar had been observed, while
ad_numbapropagates and the docstringsays it propagates. Both now propagate.
Cost, 5M rows:
rsi42.7 -> 53.8 ms,atr51.4 -> 90.3 ms,adxunchanged. NoTA-Lib exactness test moved.
A rolling kernel compiled inside its own window
Found while investigating a downstream seal-check failure.
polars.Expr.rolling_mapdoes not advance its window while the callback is being compiled, so a feature whose
callback calls a lazily-jitted kernel returned the first window's value for the whole
series the first time a process evaluated it. Distinct values on the first evaluation
against the second, each in a fresh interpreter with an empty Numba cache, over a
600-row series:
regime.hurst_exponentstatistics.coefficient_of_variationstatistics.rolling_cv_zscorestatistics.rolling_driftml.rolling_entropy_lzml.rolling_entropy_pluginwarm_rolling_callbackruns the callback once on a probe window of the right lengthso compilation happens first. Applied at all ten sites with a jitted callback,
including the four that measured clean, so no site depends on which kernels another
feature happened to compile.
Tests
tests/features/test_missing_observation_handling.py- one missing bar against 18recursive and cumulative features. 23 cases fail without the fixes.
tests/features/test_rolling_map_first_call.py- each of the twelverolling_map-backed features in its own interpreter with its own
NUMBA_CACHE_DIR,first evaluation against second. 6 fail without the warm-up.
tests/features/test_microstructure.py- two Amihud tests, one hand-computing theaverage over the traded bars.
Full suite: 3913 passed, 1 skipped.
ruff check,ruff format,ty checkclean.Note for anyone bisecting: Numba's
cache=Truedoes not invalidate when a jittedcallee changes, so delete
src/**/__pycache__/*.nb[ic]after editing a kernel orthe old machine code is silently reused.