diff --git a/pypfopt/discrete_allocation.py b/pypfopt/discrete_allocation.py index f4033e0d..ffe5cbdc 100644 --- a/pypfopt/discrete_allocation.py +++ b/pypfopt/discrete_allocation.py @@ -297,7 +297,7 @@ def lp_portfolio(self, reinvest=False, verbose=False, solver=None): print error analysis? Defaults to False. solver : str, optional the CVXPY solver to use (must support mixed-integer programs). - Defaults to "ECOS_BB" if ecos is installed, else None. + Defaults to "ECOS_BB" when that solver is installed, else None. Returns ------- @@ -306,7 +306,13 @@ def lp_portfolio(self, reinvest=False, verbose=False, solver=None): of funds leftover. """ # todo 1.7.0: remove this defaulting behavior - if solver is None and _check_soft_dependencies("ecos", severity="none"): + # The ecos distribution can be present while cvxpy does not register + # ECOS_BB. Selecting that missing solver raises SolverError. + if ( + solver is None + and _check_soft_dependencies("ecos", severity="none") + and "ECOS_BB" in cp.installed_solvers() + ): solver = "ECOS_BB" warn( "The default solver for lp_portfolio will change from ECOS_BB to" diff --git a/tests/test_discrete_allocation.py b/tests/test_discrete_allocation.py index f0953186..f1d11207 100644 --- a/tests/test_discrete_allocation.py +++ b/tests/test_discrete_allocation.py @@ -1,3 +1,4 @@ +import cvxpy as cp from cvxpy.error import SolverError import numpy as np import pandas as pd @@ -223,6 +224,102 @@ def test_greedy_portfolio_allocation_short_different_params_reinvest(): np.testing.assert_almost_equal(long_total + short_total + leftover, 100000) +@pytest.fixture +def patched_lp_solve(monkeypatch): + """Patch the ecos check, installed solvers, and ``Problem.solve`` together. + + The ecos soft-dependency check is forced true. ``installed`` is the list + ``installed_solvers`` should return, or a callable that returns that list. + The solver argument passed to ``solve`` is stored on the returned dict. + ``forward=False`` raises ``SolverError`` after recording; ``forward=True`` + calls the real solver. ``real_installed_solvers`` is the unpatched function. + """ + requested = {} + real_solve = cp.Problem.solve + real_installed_solvers = cp.installed_solvers + + monkeypatch.setattr( + "pypfopt.discrete_allocation._check_soft_dependencies", + lambda *args, **kwargs: True, + ) + + def configure(installed, *, forward=False): + def installed_solvers(): + if callable(installed): + return installed() + return list(installed) + + def solve(self, *args, **kwargs): + requested["solver"] = kwargs.get("solver") + if not forward: + raise SolverError("stop before a real solve") + return real_solve(self, *args, **kwargs) + + monkeypatch.setattr( + "pypfopt.discrete_allocation.cp.installed_solvers", + installed_solvers, + ) + monkeypatch.setattr( + "pypfopt.discrete_allocation.cp.Problem.solve", + solve, + ) + return requested + + configure.real_installed_solvers = real_installed_solvers + return configure + + +def test_lp_portfolio_skips_ecos_bb_when_cvxpy_removed_it(patched_lp_solve): + requested = patched_lp_solve(["OSQP", "SCS"]) + + prices = pd.Series({"A": 10.0, "B": 20.0}) + da = DiscreteAllocation({"A": 0.5, "B": 0.5}, prices, total_portfolio_value=100) + with pytest.raises(SolverError): + da.lp_portfolio() + assert requested["solver"] is None + + +def test_lp_portfolio_still_defaults_to_ecos_bb_when_installed(patched_lp_solve): + requested = patched_lp_solve(["ECOS_BB", "OSQP"]) + + prices = pd.Series({"A": 10.0, "B": 20.0}) + da = DiscreteAllocation({"A": 0.5, "B": 0.5}, prices, total_portfolio_value=100) + with pytest.warns(FutureWarning, match="ECOS_BB"): + with pytest.raises(SolverError): + da.lp_portfolio() + assert requested["solver"] == "ECOS_BB" + + +def test_lp_portfolio_solves_when_ecos_imports_but_ecos_bb_is_unregistered( + patched_lp_solve, +): + """Solve when the ecos check passes but cvxpy does not list ECOS_BB. + + ``installed_solvers`` is patched to drop ECOS_BB and the ecos + soft-dependency check is forced true. main still requests ECOS_BB, + so this fails there. Here the solver stays unset and a two-asset + allocation solves. + """ + + def installed_solvers_without_ecos_bb(): + return [ + name + for name in patched_lp_solve.real_installed_solvers() + if name != "ECOS_BB" + ] + + requested = patched_lp_solve(installed_solvers_without_ecos_bb, forward=True) + + prices = pd.Series({"A": 10.0, "B": 20.0}) + da = DiscreteAllocation({"A": 0.5, "B": 0.5}, prices, total_portfolio_value=1000) + allocation, leftover = da.lp_portfolio() + + assert requested["solver"] is None + assert allocation["A"] * 10 + allocation["B"] * 20 + leftover == pytest.approx(1000) + assert allocation["A"] > 0 + assert allocation["B"] > 0 + + def test_lp_portfolio_allocation(): ef = setup_efficient_frontier() w = ef.max_sharpe(risk_free_rate=0.02)