Repository navigation
Conversation
- add `signals.neutral_churn` to rank-Gaussianize and neutralize each era before calculating Spearman churn - add `signals.calculate_mean_neutral_churn` to average neutral churn across the provided recent history, using the same datestamp-keyed submissions, neutralizers, sample weights, and full-universe cleanup as the turnover threshold - add `signals.neutral_churn_penalty` for the capped positive-payout retention curve `min(1, 2 / (1 + exp(10 * (x - 0.1))))` - bump `numerai-tools` to version `0.7.1` and document these Signals helpers under the matching changelog section - document the public helpers and cover the transformation, recent-history mean, empty-history fallback, curve boundaries, monotonic decay, overflow safety, and invalid inputs
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it introduces new payout-affecting statistical logic (neutral churn, its logistic decay penalty, and a refactored submission-cleaning helper shared across two churn calculations), a human look would still be worthwhile.
What was reviewed: the neutral_churn/_neutralize_signal tie-rank-Gaussianize-neutralize pipeline and its churn composition; the neutral_churn_penalty logistic curve's boundary/overflow handling; the _clean_signal_submission refactor and its reuse in both calculate_max_churn_and_turnover and calculate_mean_neutral_churn; and the differing AssertionError-swallowing behavior between the two churn functions (calculate_mean_neutral_churn swallows any AssertionError from churn(), while calculate_max_churn_and_turnover now re-raises unexpected ones) — confirmed intentional and covered by dedicated tests, not a regression.
Extended reasoning...
Overview
The diff adds three new public functions to numerai_tools/signals.py (neutral_churn, neutral_churn_penalty, calculate_mean_neutral_churn) plus a private helper _neutralize_signal and a factored-out _clean_signal_submission used by both the new mean-churn function and the existing calculate_max_churn_and_turnover. Docs (README, CHANGELOG) and the version bump in pyproject.toml are updated to match, and tests/test_signals.py gains substantial coverage for the new code paths, including boundary conditions of the logistic penalty curve and the AssertionError-swallowing behavior of both churn-aggregation functions.
Security risks
None. This is pure numerical/statistical logic operating on pandas Series/DataFrames already validated via validate_submission_signals/clean_submission; there is no I/O, deserialization, auth, or external input handling introduced.
Level of scrutiny
Moderate-to-high: while the change is well-tested and self-contained, it implements a new payout-penalty formula (Signals v3 churn penalty) that will presumably affect real payout calculations once wired up elsewhere, and it touches shared/refactored code (_clean_signal_submission) used by an existing production function (calculate_max_churn_and_turnover). A domain expert should confirm the penalty curve formula and the intentional asymmetry in error handling between the two churn-aggregation functions match the intended Signals v3 spec.
Other factors
Test coverage is strong: the new tests directly exercise the curve boundaries, monotonicity, overflow safety, invalid-input assertions, the empty-history fallback, and (via mocking) the differing AssertionError-swallowing semantics between calculate_mean_neutral_churn and calculate_max_churn_and_turnover. The four candidate issues previously investigated (broad AssertionError swallowing in the new mean-churn path, missing per-datestamp try/except, the unconditional std-dev assert versus the "1.0 when no comparison" docstring claim, and NaN-dropping tolerance in _neutralize_signal) all check out as either matching the sibling function's existing pattern or intentional/tested design choices rather than clear-cut bugs, so I'm not raising them independently.
1 participant
signals.neutral_churnto rank-Gaussianize and neutralize each era before calculating Spearman churnsignals.calculate_mean_neutral_churnto average neutral churn across the provided recent history, using the same datestamp-keyed submissions, neutralizers, sample weights, and full-universe cleanup as the turnover thresholdsignals.neutral_churn_penaltyfor the capped positive-payout retention curvemin(1, 2 / (1 + exp(10 * (x - 0.1))))numerai-toolsto version0.7.1and document these Signals helpers under the matching changelog section