Skip to content

Apply fully-invested risk lower bound only to fully-invested portfolios - #770

Open
kadyrbekovhamit-cyber wants to merge 1 commit into
PyPortfolio:mainfrom
kadyrbekovhamit-cyber:fix/market-neutral-risk-feasibility
Open

kadyrbekovhamit-cyber wants to merge 1 commit into
PyPortfolio:mainfrom
kadyrbekovhamit-cyber:fix/market-neutral-risk-feasibility

Conversation

@kadyrbekovhamit-cyber

Copy link
Copy Markdown

efficient_risk(..., market_neutral=True) currently applies the sum(weights)=1 global-minimum-volatility lower bound before solving the sum(weights)=0 problem. That rejects feasible low-risk neutral portfolios.

For returns [.1,0], covariance .04*I, bounds (-1,1) and target volatility .05, weights [.05/sqrt(.08),-.05/sqrt(.08)] are feasible and analytically optimal. The current precheck rejects them because the fully-invested lower bound is sqrt(.02).

Apply that precheck only when market_neutral=False; the solver still evaluates all user constraints. Tests cover three low-risk targets, updating the target parameter, preservation of the fully-invested rejection and a genuinely infeasible neutral constraint.

Validation: original regressions 5 failed / 1 passed; patched efficient-frontier module plus regressions 77 passed / 4 skipped. Main a6638d2; Python 3.12, cvxpy 1.9.3. Related #569 discusses minimum-volatility rejections but does not supply this market-neutral counterexample; #362 concerns sector constraints, which this patch does not change.

AI-assisted investigation and patch; numerical work was sequential and single-threaded. This is a feasibility-check correction, not a claim about production portfolios.

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

Copy link
Copy Markdown

I checked out this branch (Python 3.12, cvxpy 1.9.3) and ran the two-asset case from the description with efficient_risk(0.05, market_neutral=True). With bounds (-1, 1) it returns [0.17678, -0.17678], which equals 0.05/sqrt(0.08), so the precheck removal does what the PR says.

Two things I'd add tests for, since the precheck was the only thing that stopped some bad inputs early:

  1. Bounds that cap the weights below the analytic solution, e.g. (-0.02, 0.02) with target 0.05. I get [0.02, -0.02] (feasible, volatility below the target, because the constraint is <=). That is fine, but it is not in the test file and the other tests all use wide bounds where the answer is exactly [a, -a], so a future change that tightens the constraint to equality would not be caught.
  2. Bounds that exclude negatives, e.g. (0.1, 1). The existing code warns "Market neutrality requires shorting - bounds have been amended" and solves anyway (I get [0.1768, -0.1768]). Unrelated to this PR, but the test for the infeasible case only covers an added constraint, so a note on whether the amended-bounds path is intended here would help reviewers.

The if not market_neutral placement looks right to me: the 1/sum(pinv(cov)) value is the minimum for sum(w)=1 only, and sum(w)=0 has zero as feasible risk.

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