Skip to content

Handle missing return observations explicitly in shrinkage and semicovariance - #771

Open
kadyrbekovhamit-cyber wants to merge 1 commit into
PyPortfolio:mainfrom
kadyrbekovhamit-cyber:fix/missing-return-estimation
Open

kadyrbekovhamit-cyber wants to merge 1 commit into
PyPortfolio:mainfrom
kadyrbekovhamit-cyber:fix/missing-return-estimation

Conversation

@kadyrbekovhamit-cyber

Copy link
Copy Markdown

Automatic shrinkage estimators currently replace missing returns with zero. semicovariance also converts NaNs to zero via fmin and divides every pair by the full row count. An unobserved return is therefore treated as an observed flat day.

Proposed policy:

  • Ledoit–Wolf (all three targets) and OAS use the common complete-return sample, warn with the number of dropped rows, and require at least two complete observations. Constant-correlation covariance and higher moments use that same sample, retaining the existing ddof convention. Manual shrunk_covariance(delta) remains unchanged.
  • Semicovariance divides each downside-product sum by that asset pair's joint-observation count, rejects pairs with no overlap, and retains the existing PSD-repair option. Actual zero returns still count as observations.

This makes a statistical choice explicit: complete-case shrinkage can discard much of the older history; pairwise estimates can need PSD repair. The docs state these limitations. I would welcome feedback on the policy/API before merge.

Six old integration goldens change because they included the zero-imputation result. I cross-checked the updated moments with pairwise means and sklearn, the BL posterior with its precision-form expression, and five optimized objective values with an independent scipy SLSQP solve (largest objective gap below 5e-13), before updating those goldens.

Validation: original initial regressions 14 failed / 1 passed; patched risk_models + 16 regressions + Black–Litterman/CLA/efficient-frontier modules: 146 passed / 4 skipped. Tests include internal/leading gaps, input immutability, insufficient overlap, observed zeros and off-diagonal joint counts. On the repository stock-price fixture at benchmark=0, BABA downside variance is .00560896 with the old denominator versus .04465234 using observed days; the long-only minimum-risk weight changes from .532475 to .081924. This is a fixture example, not observed investment losses or a universal multiplier.

Related prior discussion: #270 (common history recommendation), #294 (zero-fill criticism in a different estimator), #500/#514 (new-stock overweight symptoms). I do not claim that the general missing-data issue is new. No matching current patch found in the reviewed open PRs. Tested main a6638d2, Python 3.12 / numpy 2.2.6 / pandas 3.0.6. AI-assisted investigation and patch, one numerical worker.

…variance

Signed-off-by: kadyrbekovhamit-cyber <288885044+kadyrbekovhamit-cyber@users.noreply.github.com>
@arhancanli

Copy link
Copy Markdown

Read the diff (did not run the suite). The semicovariance change divides each pair by its joint observation count instead of T, and the updated expectations show how much that moves results on the repo's test prices: test_max_sharpe_semicovariance goes from volatility 0.0737 / Sharpe 3.48 to 0.1733 / 1.81, and test_cla_max_sharpe_semicovariance from 0.0726 / 3.47 to 0.1707 / 1.81. That is a 2x change in reported risk for existing users of risk_models.semicovariance on data with gaps, so it deserves a line in the PR description and the release notes, not only a docs section. The old divisor-T version effectively treated missing returns as zero drops and shrank the matrix; the new one is closer to right, but anyone with a saved max-Sharpe result will see it change.

One question the tests could answer: with the pairwise counts, entries are averaged over different windows, so the matrix is not guaranteed PSD (the docs mention the spectral fix). Is there a test showing fix_nonpositive_semidefinite actually fires on a gappy input and by how much it moves the diagonal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants