Skip to content

fix(riskmodel): keep structured covariance on one sample denominator - #2378

Open
Divyansh Barodiya (Divyansh151005) wants to merge 2 commits into
microsoft:mainfrom
Divyansh151005:fix/2377-structured-cov-moments
Open

Divyansh Barodiya (Divyansh151005) wants to merge 2 commits into
microsoft:mainfrom
Divyansh151005:fix/2377-structured-cov-moments

Conversation

@Divyansh151005

@Divyansh151005 Divyansh Barodiya (Divyansh151005) commented Oct 17, 2026 •

Copy link
Copy Markdown

Description

StructuredCovEstimator estimates a statistical factor covariance

cov(X) = F @ cov(B) @ F.T + diag(var(U))

Two moment conventions in _predict were inconsistent with that formula.

np.cov on a single factor returns a scalar. F @ cov_b @ F.T then raises ValueError, and return_decomposed_components=True hands the same scalar to examples/portfolio/prepare_riskdata.py as factor_cov. The factor covariance is now always (num_factors, num_factors), including one factor.

np.cov divides by N - 1. np.var divides by N, so residual variance was smaller than the factor block by (N - 1) / N. For a PCA fit the scores are orthogonal to the residuals, and the diagonal of the structured covariance should equal the sample variances of the returns. Residual variance now uses ddof=1.

Motivation and Context

Fixes #2377

Enhanced indexing reads this decomposition as factor exposures, factor covariance, and specific risk. A one-factor model could not form the covariance, and a shorter window understated idiosyncratic variance (about 5% on 20 observations, about 1.7% on 60). The existing tests compare the estimator to np.cov only when every component is kept, or within 0.1, so neither case was covered.

How Has This Been Tested?

  • Pass the test by running: pytest qlib/tests/test_all_pipeline.py under upper directory of qlib.
  • If you are adding a new feature, test on your own test scripts.

test_all_pipeline.py trains a CSI300 LightGBM workflow and needs the market dataset. These tests use a synthetic return panel.

$ python -m pytest tests/test_structured_cov_estimator.py -q --tb=short
.......                                                                  [100%]
7 passed in 3.86s

Before the change, num_factors=1 raised ValueError: matmul: Input operand 1 does not have enough dimensions, and the PCA diagonal differed from np.cov by 3.2e-2 on a 20×4 panel with 2 factors. After the change that diagonal difference is about 1e-15.

flake8 on the changed Python files, with the ignore list from the developer guide, reported no issues. black -l 120 left the files unchanged.

Screenshots of Test Results (if appropriate):

  1. Pipeline test: not run. It needs the CSI300 dataset and a model fit.
  2. Your own tests: tests/test_structured_cov_estimator.py, 7 passed.

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

A single factor made np.cov return a scalar, so the covariance could not be formed. Residual variance also divided by N while the factor block divided by N - 1, which understated specific risk.
@arhancanli

Copy link
Copy Markdown

Checked the algebra: for the PCA fit, X = B F^T + U with B orthogonal to U and both centred, so diag(F cov(B) F^T) + var(U) equals var(X) column by column only if both moments share a denominator. With np.var at ddof=0 against np.cov at N - 1 the diagonal was low by a factor of (N-1)/N on the residual part, so ddof=1 is the right fix and your test_pca_diagonal_matches_sample_variance pins it.

Two wording points:

  • The docstring and README call N - 1 "unbiased". That holds for the factor block, but not for the specific variance: residuals from a k-factor fit have used up k further degrees of freedom, so sum(U^2) / (N - 1) still understates the true idiosyncratic variance by roughly k / N. With N = 25, k = 2 (your test sizes) that is about 8%. "Same denominator as np.cov" is accurate; "unbiased" is not, and a reader may take it as a statement about the estimator.
  • The diagonal identity is a PCA property. For factor_model="fa" the residuals are not orthogonal to the scores, so the diagonal is not guaranteed to equal the sample variance. Saying "for PCA" in the README sentence (it is only in the class docstring) avoids someone testing it on FA and filing a bug.

The np.atleast_2d fix for one factor looks right; np.cov of a single row returns a 0-d array, and the 1x1 shape assertion covers it.

N - 1 matches numpy.cov. It is not an unbiased specific variance after a k-factor fit, and the diagonal identity holds for PCA only.
@Divyansh151005

Copy link
Copy Markdown
Author

Thanks for checking the algebra. I pushed a wording follow-up.

The docstring and the portfolio README no longer call N - 1 unbiased. They say both moments use the same denominator as numpy.cov, and that a k-factor residual still has fewer degrees of freedom than that. The diagonal identity is described as a PCA property. The README now says factor analysis does not have it.

The one-factor np.atleast_2d change is unchanged.

$ python -m pytest tests/test_structured_cov_estimator.py -q --tb=line
.......                                                                  [100%]

@koriyoshi2041

Copy link
Copy Markdown

Verified the wording follow-up at aa4187a1. The docstring and portfolio README now scope the diagonal identity to PCA and describe N - 1 as the shared numpy.cov convention rather than an unbiased specific-variance estimate.

The exact-head risk-model test file passes 7/7, including the PCA diagonal and one-factor PCA/FA shape regressions. I also checked the two-commit diff for whitespace errors; none found.

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.

StructuredCovEstimator crashes for one factor and uses inconsistent residual variance

3 participants