Add assume_PSD=True to ex_ante_tracking_error's quad_form - #757
Open
shlokkvaishnav wants to merge 1 commit into
Open
Add assume_PSD=True to ex_ante_tracking_error's quad_form#757shlokkvaishnav wants to merge 1 commit into
shlokkvaishnav wants to merge 1 commit into
Conversation
portfolio_variance, sharpe_ratio and quadratic_utility all call cp.quad_form(..., assume_PSD=True), skipping cvxpy's numerical PSD-certification check for the covariance matrix. This is the right call since these objectives always receive a covariance matrix, which is PSD by construction even when it's numerically ill-conditioned. ex_ante_tracking_error was the one quad_form-based objective missing this, despite taking the same kind of cov_matrix argument -- an inconsistency, and the same class of failure reported in PyPortfolio#631 for a user's custom quad_form-based objective ("failure ... trying to certify that a matrix is positive semi-definite").
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.
What this fixes
objective_functions.pyhas fourcp.quad_form(...)call sites over a covariance matrix. Three of them (portfolio_variance,sharpe_ratio,quadratic_utility) passassume_PSD=True, which skips cvxpy's own numerical PSD-certification check on the matrix — the right call, sincecov_matrixis PSD by construction even when it's numerically ill-conditioned (small sample size relative to number of assets, near-duplicate assets, etc.).ex_ante_tracking_errorwas the one outlier missing it, despite taking the same kind ofcov_matrixargument used the same way. This is the exact failure class reported in #631 (cvxpy note: ... trying to certify that a matrix is positive semi-definite ... replace the matrix A by cvxpy.psd_wrap(A)), for a user's ownquad_form-based objective modeled on this codebase's pattern.Honesty about verification
I was not able to force this exact PSD-certification error against the current cvxpy (1.9.2) with any covariance matrix I constructed (including exactly-singular, ill-conditioned, and rank-deficient ones) — cvxpy appears to have become more lenient about this over time. So this isn't a "fails on main, passes with fix" reproduction; it's a defensive consistency fix that matches the established pattern used by the sibling objective functions in this same file, and directly addresses the failure mode described in #631.
Test log
Added
test_ex_ante_tracking_error_assumes_psdconfirming the numeric output is unchanged for a well-conditioned matrix (i.e. the fix is behavior-preserving in the normal case), alongside the existingtest_ex_ante_tracking_error/test_ex_ante_tracking_error_dummytests which both still pass unmodified.