diff --git a/CLAUDE.md b/CLAUDE.md index 8c3cd329a..8112a8190 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -103,6 +103,15 @@ guards inputs by walking `Model.keywords`, which defaults to the keys of or a caller who omits that keyword gets a bare `KeyError` rather than a `CvxDataError`. +**A model must also declare the dimensions its data implies.** +`Model.dimensions` is abstract for the same reason `keywords` exists: `update` +zero-pads short input up to the compiled size, so a payload that describes two +assets to the risk model and four to the bounds does not fail on its own -- it +leaves the padded tail riskless and unbounded and the solver empties the +portfolio into it. `Problem.update` collects the `(variable, size)` claims of +every model and rejects the payload before writing a single value. A model that +consumes a keyword without declaring its size reopens that hole silently. + **Everything raised derives from `CvxError`.** `CvxDataError`, `CvxBuildError` and `CvxSolverError` split the failure modes by whether retrying helps; the table in `README.md` is the contract. `tests/test_markowitz/test_public_api.py` diff --git a/README.md b/README.md index ac098fc96..c6817913a 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,14 @@ the number of assets fixed by setting the weights for the assets not used to zero. Hence we do **not** need to recompile the problem as a new asset has to be added. +The padding is one-directional and it has to be consistent. Data for *more* +assets than the problem was built for does not fit and raises a `CvxDataError`, +and so does a payload whose models disagree about how large the universe is -- +handing the risk model two assets and the bounds four would otherwise leave the +padded tail both riskless and unbounded, and the solver would put the whole +portfolio there. `update` checks this across all models before it writes +anything. + Every problem has to be constructed by a Builder. Here's a builder for a classic [minimum variance problem](src/cvxmarkowitz/portfolios/min_var.py). The builder inherits from the [Builder](src/cvxmarkowitz/builder.py) diff --git a/src/cvxmarkowitz/model.py b/src/cvxmarkowitz/model.py index 18172fc9a..80953e4f1 100644 --- a/src/cvxmarkowitz/model.py +++ b/src/cvxmarkowitz/model.py @@ -20,7 +20,7 @@ import cvxpy as cp -from cvxmarkowitz.types import Constraints, Matrix, Parameter, Variables +from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Parameter, Variables @dataclass(frozen=True) @@ -79,6 +79,29 @@ def estimate(self, variables: Variables) -> cp.Expression: -- see `Bounds`, which is pure constraints. """ + @abstractmethod + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the size every input in `kwargs` implies for a problem variable. + + One `(variable name, size)` pair per input this model consumes, the + variable named by `DataNames` -- `WEIGHTS` for anything sized by the + asset universe, `FACTOR_WEIGHTS` for anything sized by the factors. + Several pairs may name the same variable; that is the point. + + `Problem.update` collects these across every model and rejects a payload + whose claims disagree. It has to, because `update` pads short inputs up + to the compiled size (see `cvxmarkowitz.utils.fill`): a payload that + hands the risk model two assets and the bounds four does not fail on its + own -- it leaves the padded tail both zero-risk and unbounded, and the + solver puts the whole portfolio there. Each model already checks its own + inputs against each other; this is what checks them across models. + + Abstract, rather than a default a model may quietly not override, for + the reason `keywords` documents: that shape of contract has already been + got wrong once here. A model with nothing to declare returns `()`, but it + forfeits the cross-check for every keyword it consumes. + """ + @abstractmethod def update(self, **kwargs: Matrix) -> None: """Write fresh values into this component's parameters, in place. diff --git a/src/cvxmarkowitz/models/bounds.py b/src/cvxmarkowitz/models/bounds.py index 15c911ea5..c963de9d8 100644 --- a/src/cvxmarkowitz/models/bounds.py +++ b/src/cvxmarkowitz/models/bounds.py @@ -21,7 +21,7 @@ import numpy as np from cvxmarkowitz.model import Model -from cvxmarkowitz.types import Constraints, Matrix, Variables +from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_vector @@ -65,8 +65,21 @@ def __post_init__(self) -> None: value=np.ones(self.assets), ) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the size both bound vectors imply for the variable bounded. + + Both, not just the lower one: a payload giving a lower bound for two + assets and an upper bound for four would otherwise pad the lower bound + with zeros and leave the tail free between 0 and the real upper bound, + which is the same silent-tail failure `Problem.update` exists to catch. + """ + return ( + (self.acting_on, len(kwargs[self._f("lower")])), + (self.acting_on, len(kwargs[self._f("upper")])), + ) + def update(self, **kwargs: Matrix) -> None: - """Assign lower/upper vectors, padding or trimming to asset length.""" + """Assign lower/upper vectors, zero-padding them to the compiled length.""" self.data[self._f("lower")].value = fill_vector(num=self.assets, x=kwargs[self._f("lower")]) self.data[self._f("upper")].value = fill_vector(num=self.assets, x=kwargs[self._f("upper")]) diff --git a/src/cvxmarkowitz/models/expected_returns.py b/src/cvxmarkowitz/models/expected_returns.py index 403eee4ba..294dafdaf 100644 --- a/src/cvxmarkowitz/models/expected_returns.py +++ b/src/cvxmarkowitz/models/expected_returns.py @@ -23,7 +23,7 @@ from cvxmarkowitz.cvxerror import CvxDataError from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D -from cvxmarkowitz.types import Matrix, Variables +from cvxmarkowitz.types import Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_vector @@ -69,6 +69,13 @@ def estimate(self, variables: Variables) -> cp.Expression: """ return self.data[D.MU] @ variables[D.WEIGHTS] - self.parameter[D.MU_UNCERTAINTY] @ cp.abs(variables[D.WEIGHTS]) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the number of assets `mu` and its uncertainty imply.""" + return ( + (D.WEIGHTS, len(kwargs[D.MU])), + (D.WEIGHTS, len(kwargs[D.MU_UNCERTAINTY])), + ) + def update(self, **kwargs: Matrix) -> None: """Update expected returns and their uncertainty bounds. diff --git a/src/cvxmarkowitz/models/holding_costs.py b/src/cvxmarkowitz/models/holding_costs.py index f78743a52..779586908 100644 --- a/src/cvxmarkowitz/models/holding_costs.py +++ b/src/cvxmarkowitz/models/holding_costs.py @@ -22,7 +22,7 @@ from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D -from cvxmarkowitz.types import Matrix, Variables +from cvxmarkowitz.types import Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_vector @@ -38,6 +38,10 @@ def estimate(self, variables: Variables) -> cp.Expression: """Return total holding costs as -sum(w_i * c_i).""" return cp.sum(cp.neg(cp.multiply(variables[D.WEIGHTS], self.data[D.HOLDING_COSTS]))) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the number of assets the holding-cost vector implies.""" + return ((D.WEIGHTS, len(kwargs[D.HOLDING_COSTS])),) + def update(self, **kwargs: Matrix) -> None: """Update the holding-cost vector from kwargs[D.HOLDING_COSTS].""" self.data[D.HOLDING_COSTS].value = fill_vector(num=self.assets, x=kwargs[D.HOLDING_COSTS]) diff --git a/src/cvxmarkowitz/models/trading_costs.py b/src/cvxmarkowitz/models/trading_costs.py index fd2d1770a..9bcd471cb 100644 --- a/src/cvxmarkowitz/models/trading_costs.py +++ b/src/cvxmarkowitz/models/trading_costs.py @@ -23,7 +23,7 @@ from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D from cvxmarkowitz.names import ParameterName as P -from cvxmarkowitz.types import Matrix, Variables +from cvxmarkowitz.types import Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_vector @@ -56,6 +56,10 @@ def estimate(self, variables: Variables) -> cp.Expression: ) ) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the number of assets the previous weights imply.""" + return ((D.WEIGHTS, len(kwargs[D.WEIGHTS])),) + def update(self, **kwargs: Matrix) -> None: """Update cached data values. diff --git a/src/cvxmarkowitz/problem.py b/src/cvxmarkowitz/problem.py index 9361010ca..35f6dc0d2 100644 --- a/src/cvxmarkowitz/problem.py +++ b/src/cvxmarkowitz/problem.py @@ -53,11 +53,45 @@ def update(self, **kwargs: Matrix) -> None: the same object do not yield two independently parametrized problems -- the second overwrites the first. Call `build()` again for that. + The whole payload is validated against every model before the first + value is written, so a rejected payload leaves the problem exactly as it + was rather than half-overwritten. See `_validate`. + Returns `None` (like `Model.update`) so the in-place semantics are visible at the call site. Raises: - CvxDataError: If any model is missing data for one of its parameters. + CvxDataError: If any model is missing data for one of its parameters, + or if the models disagree about how large the universe is. + """ + self._validate(**kwargs) + + for model in self.model.values(): + # It's tempting to operate without the models at this stage. + # However, we would give up a lot of convenience. For example, + # the models can be prepared to deal with data that has not + # exactly the correct shape. + model.update(**kwargs) + + def _validate(self, **kwargs: Matrix) -> None: + """Check the payload against every model, writing nothing. + + Two passes, both over all models, both raising `CvxDataError`: + + 1. every keyword each model declares is present, and + 2. the models agree on the size of each variable they describe. + + The second is not redundant with the shape checks inside the models. + `Model.update` pads a short input up to the compiled size, so a payload + that describes two assets to the risk model and four to the bounds + solves without complaint -- and solves wrongly, because the padded tail + carries no risk while the bounds leave it free, which the solver reads as + two riskless assets. Nothing inside a single model can see that; only + comparing the models can. + + Raises: + CvxDataError: On a missing keyword, or on models that disagree about + the size of a variable. """ for name, model in self.model.items(): # `Model.keywords`, not `model.data`: a model may consume a keyword @@ -67,11 +101,17 @@ def update(self, **kwargs: Matrix) -> None: if key not in kwargs: raise CvxDataError(f"Missing data for {key} in model {name}") # noqa: TRY003 - # It's tempting to operate without the models at this stage. - # However, we would give up a lot of convenience. For example, - # the models can be prepared to deal with data that has not - # exactly the correct shape. - model.update(**kwargs) + claimed: dict[str, tuple[str, int]] = {} + + for name, model in self.model.items(): + for variable, size in model.dimensions(**kwargs): + first_name, first_size = claimed.setdefault(variable, (name, size)) + + if size != first_size: + raise CvxDataError( # noqa: TRY003 + f"Inconsistent size for {variable}: model {first_name} was given " + f"{first_size}, model {name} was given {size}" + ) def solve(self, solver: str = cp.CLARABEL, **kwargs: Any) -> float: """Solve the problem.""" diff --git a/src/cvxmarkowitz/risk/cvar/cvar.py b/src/cvxmarkowitz/risk/cvar/cvar.py index 0b7c5e599..da0c62278 100644 --- a/src/cvxmarkowitz/risk/cvar/cvar.py +++ b/src/cvxmarkowitz/risk/cvar/cvar.py @@ -23,7 +23,7 @@ from cvxmarkowitz.cvxerror import CvxDataError from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D -from cvxmarkowitz.types import Matrix, Variables +from cvxmarkowitz.types import Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_matrix @@ -74,6 +74,15 @@ def estimate(self, variables: Variables) -> cp.Expression: # average value of the k elements in the left tail return -cp.sum_smallest(self.data[D.RETURNS] @ variables[D.WEIGHTS], k=k) / k + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the number of assets the scenario matrix implies. + + Its row count is the number of scenarios, which is this model's own + business rather than a size shared with the other models, so it is not + declared here. + """ + return ((D.WEIGHTS, np.shape(kwargs[D.RETURNS])[1]),) + def update(self, **kwargs: Matrix) -> None: """Update the returns matrix used by the CVaR model. diff --git a/src/cvxmarkowitz/risk/factor/factor.py b/src/cvxmarkowitz/risk/factor/factor.py index cc8f69d69..f04abf466 100644 --- a/src/cvxmarkowitz/risk/factor/factor.py +++ b/src/cvxmarkowitz/risk/factor/factor.py @@ -23,7 +23,7 @@ from cvxmarkowitz.cvxerror import CvxDataError from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D -from cvxmarkowitz.types import Constraints, Matrix, Variables +from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_matrix, fill_vector @@ -113,6 +113,21 @@ def systematic_risk(self, variables: Variables) -> cp.Expression: ) ) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the asset and factor counts every factor-model input implies.""" + factors, assets = np.shape(kwargs[D.EXPOSURE]) + chol_rows, chol_cols = np.shape(kwargs[D.CHOLESKY]) + + return ( + (D.WEIGHTS, assets), + (D.WEIGHTS, len(kwargs[D.IDIOSYNCRATIC_VOLA])), + (D.WEIGHTS, len(kwargs[D.IDIOSYNCRATIC_VOLA_UNCERTAINTY])), + (D.FACTOR_WEIGHTS, factors), + (D.FACTOR_WEIGHTS, chol_rows), + (D.FACTOR_WEIGHTS, chol_cols), + (D.FACTOR_WEIGHTS, len(kwargs[D.SYSTEMATIC_VOLA_UNCERTAINTY])), + ) + def update(self, **kwargs: Matrix) -> None: """Validate and assign all factor-model inputs. diff --git a/src/cvxmarkowitz/risk/sample/sample.py b/src/cvxmarkowitz/risk/sample/sample.py index f19469c15..6c86109de 100644 --- a/src/cvxmarkowitz/risk/sample/sample.py +++ b/src/cvxmarkowitz/risk/sample/sample.py @@ -23,7 +23,7 @@ from cvxmarkowitz.cvxerror import CvxDataError from cvxmarkowitz.model import Model from cvxmarkowitz.names import DataNames as D -from cvxmarkowitz.types import Constraints, Matrix, Variables +from cvxmarkowitz.types import Constraints, Dimensions, Matrix, Variables from cvxmarkowitz.utils.fill import fill_matrix, fill_vector @@ -58,6 +58,16 @@ def estimate(self, variables: Variables) -> cp.Expression: ) ) + def dimensions(self, **kwargs: Matrix) -> Dimensions: + """Return the number of assets the Cholesky factor and uncertainty imply.""" + rows, cols = np.shape(kwargs[D.CHOLESKY]) + + return ( + (D.WEIGHTS, rows), + (D.WEIGHTS, cols), + (D.WEIGHTS, len(kwargs[D.VOLA_UNCERTAINTY])), + ) + def update(self, **kwargs: Matrix) -> None: """Assign Cholesky factor and volatility-uncertainty vector. diff --git a/src/cvxmarkowitz/types.py b/src/cvxmarkowitz/types.py index dafc809f7..0f20ee5bd 100644 --- a/src/cvxmarkowitz/types.py +++ b/src/cvxmarkowitz/types.py @@ -24,3 +24,9 @@ Constraints = dict[str, cp.Constraint] Matrix: TypeAlias = npt.NDArray[np.float64] + +# What `Model.dimensions` reports: (variable name, size) claims, one per input +# the model consumes. A tuple rather than a mapping because several inputs of +# one model speak about the same variable, and it is exactly their +# disagreement that `Problem.update` is looking for. +Dimensions: TypeAlias = tuple[tuple[str, int], ...] diff --git a/src/cvxmarkowitz/utils/fill.py b/src/cvxmarkowitz/utils/fill.py index 0d4aeea37..e7a9e10c4 100644 --- a/src/cvxmarkowitz/utils/fill.py +++ b/src/cvxmarkowitz/utils/fill.py @@ -15,20 +15,49 @@ import numpy as np +from cvxmarkowitz.cvxerror import CvxDataError from cvxmarkowitz.types import Matrix def fill_vector(x: Matrix, num: int) -> Matrix: - """Fill a vector of length num with x.""" + """Return a vector of length ``num`` holding ``x`` in its leading entries. + + The tail is zero. This is what lets one compiled problem serve a universe + smaller than the one it was built for: `Bounds` pads both of its bounds + this way, which pins the unused tail to ``0 <= w <= 0``. + + Padding only ever goes one way. An ``x`` longer than ``num`` does not fit + the compiled problem at all, so it is reported as a `CvxDataError` rather + than truncated silently or left to escape as the `ValueError` numpy raises + on the assignment below -- `CvxDataError` is the failure mode the README + promises for input whose shapes do not fit. + + Raises: + CvxDataError: If ``x`` is longer than ``num``. + """ + if len(x) > num: + raise CvxDataError(f"Vector of length {len(x)} does not fit a problem built for {num}") # noqa: TRY003 + z = np.zeros(num) z[: len(x)] = x return z def fill_matrix(x: Matrix, rows: int, cols: int) -> Matrix: - """Fill a matrix of size (rows, cols) with x.""" + """Return a ``rows`` x ``cols`` matrix holding ``x`` in its top-left block. + + The counterpart of `fill_vector`; see there for why the padding is only + ever one-directional. + + Raises: + CvxDataError: If ``x`` does not fit into ``(rows, cols)``. + """ # I had no luck with ndarray.resize() - z = np.zeros((rows, cols)) (n, m) = np.shape(x) + + if n > rows or m > cols: + raise CvxDataError(f"Matrix of shape {(n, m)} does not fit a problem built for {(rows, cols)}") # noqa: TRY003 + + z = np.zeros((rows, cols)) z[:n, :m] = x return z diff --git a/tests/test_markowitz/test_models/test_holding_costs.py b/tests/test_markowitz/test_models/test_holding_costs.py index d20a8efa1..4396db44f 100644 --- a/tests/test_markowitz/test_models/test_holding_costs.py +++ b/tests/test_markowitz/test_models/test_holding_costs.py @@ -25,3 +25,16 @@ def test_holding_costs(): variables = {D.WEIGHTS: weights} assert model.estimate(variables).value == pytest.approx(0.04) + + +def test_dimensions(): + """The model reports the universe its holding-cost vector describes. + + HoldingCosts pads a short vector to the compiled length like every other + model, so it has to declare what it was given -- otherwise a problem + carrying it could be handed a two-asset cost vector and four-asset bounds + without `Problem.update` noticing. + """ + model = HoldingCosts(assets=3) + + assert model.dimensions(**{D.HOLDING_COSTS: np.array([0.1, 0.2])}) == ((D.WEIGHTS, 2),) diff --git a/tests/test_markowitz/test_models/test_trading_costs.py b/tests/test_markowitz/test_models/test_trading_costs.py index ff382241c..86cdf6cfb 100644 --- a/tests/test_markowitz/test_models/test_trading_costs.py +++ b/tests/test_markowitz/test_models/test_trading_costs.py @@ -26,3 +26,14 @@ def test_trading_costs(): variables = {D.WEIGHTS: weights} assert model.estimate(variables).value == pytest.approx(0.8) + + +def test_dimensions(): + """The model reports the universe its previous weights describe. + + Keyed by `D.WEIGHTS` in both halves of the pair: the previous weights are + the previous value of the weight variable, and are sized by it. + """ + model = TradingCosts(assets=3) + + assert model.dimensions(**{D.WEIGHTS: np.array([0.1, 0.2])}) == ((D.WEIGHTS, 2),) diff --git a/tests/test_markowitz/test_portfolios/test_problem.py b/tests/test_markowitz/test_portfolios/test_problem.py index e89614523..28427b14a 100644 --- a/tests/test_markowitz/test_portfolios/test_problem.py +++ b/tests/test_markowitz/test_portfolios/test_problem.py @@ -9,6 +9,7 @@ from cvxmarkowitz import CvxDataError, MaxSharpe, MinVar from cvxmarkowitz.names import DataNames as D +from cvxmarkowitz.names import ModelName as M def _data(correlation: float) -> dict[str, np.ndarray]: @@ -38,6 +39,21 @@ def _max_sharpe_data() -> dict[str, np.ndarray]: } +def _factor_data(assets: int = 3, factors: int = 2) -> dict[str, np.ndarray]: + """Return a complete payload for a MinVar problem with a factor risk model.""" + return { + D.EXPOSURE: np.ones((factors, assets)), + D.CHOLESKY: np.eye(factors), + D.IDIOSYNCRATIC_VOLA: np.full(assets, 0.1), + D.IDIOSYNCRATIC_VOLA_UNCERTAINTY: np.zeros(assets), + D.SYSTEMATIC_VOLA_UNCERTAINTY: np.zeros(factors), + D.LOWER_BOUND_ASSETS: np.zeros(assets), + D.UPPER_BOUND_ASSETS: np.ones(assets), + D.LOWER_BOUND_FACTORS: -np.ones(factors), + D.UPPER_BOUND_FACTORS: np.ones(factors), + } + + def test_problem_data(): """get_problem_data returns the compiled data, chain and inverse data.""" problem = MinVar(assets=10).build() @@ -129,3 +145,124 @@ def test_dropping_any_required_keyword_raises_cvx_data_error(): del payload[omitted] with pytest.raises(CvxDataError, match="Missing data for"): problem.update(**payload) + + +def test_a_smaller_universe_is_padded_and_solved(): + """Data for fewer assets than the problem was built for is still legal. + + This is the whole point of the padding, so it is also what the consistency + check below must not break: `Bounds` pads both bounds with zeros, which pins + the unused tail to `0 <= w <= 0`, and the answer is the two-asset one. + """ + padded = MinVar(assets=4).build() + padded.update(**_data(0.5)) + + exact = MinVar(assets=2).build() + exact.update(**_data(0.5)) + + assert padded.solve() == pytest.approx(exact.solve()) + assert padded.weights[2:] == pytest.approx(np.zeros(2), abs=1e-6) + + +def test_models_disagreeing_about_the_universe_are_rejected(): + """A payload sizing the risk model and the bounds differently is refused. + + Left to itself this solves happily and answers with nonsense: the risk model + pads its Cholesky factor with zeros, the bounds are given their full length, + and the tail becomes a set of riskless assets the solver puts everything + into. No single model can see it -- each one's own inputs are consistent. + """ + problem = MinVar(assets=4).build() + payload = _data(0.5) | {D.LOWER_BOUND_ASSETS: np.zeros(4), D.UPPER_BOUND_ASSETS: np.ones(4)} + + with pytest.raises(CvxDataError, match="Inconsistent size for weights"): + problem.update(**payload) + + +def test_the_error_names_both_disagreeing_models(): + """The message points at the two models, since the payload cannot say which is wrong.""" + problem = MinVar(assets=4).build() + payload = _data(0.5) | {D.LOWER_BOUND_ASSETS: np.zeros(4), D.UPPER_BOUND_ASSETS: np.ones(4)} + + with pytest.raises(CvxDataError) as excinfo: + problem.update(**payload) + + assert M.RISK in str(excinfo.value) + assert M.BOUND_ASSETS in str(excinfo.value) + + +def test_bounds_disagreeing_with_each_other_are_rejected(): + """A model's inputs are checked against each other too, not only across models. + + Both bounds are padded with zeros, so a short lower bound against a full + upper bound leaves the tail free between 0 and its real upper bound -- the + same riskless-tail failure, from a single model. + """ + problem = MinVar(assets=4).build() + payload = _data(0.5) | {D.UPPER_BOUND_ASSETS: np.ones(4)} + + with pytest.raises(CvxDataError, match="Inconsistent size for weights"): + problem.update(**payload) + + +def test_factor_count_disagreement_is_rejected(): + """Factors are checked as their own dimension, independently of the assets.""" + problem = MinVar(assets=3, factors=2).build() + payload = _factor_data() | { + D.LOWER_BOUND_FACTORS: -np.ones(3), + D.UPPER_BOUND_FACTORS: np.ones(3), + } + + with pytest.raises(CvxDataError, match=f"Inconsistent size for {D.FACTOR_WEIGHTS}"): + problem.update(**payload) + + +def test_a_consistent_factor_payload_still_passes(): + """The factor problem's own dimensions agree, so a good payload is untouched. + + With an exposure of ones every budgeted portfolio has the same factor + weights, so the systematic risk is fixed at ``sqrt(2)`` and only the + idiosyncratic part is left to minimise -- which the equal-weight portfolio + does. + """ + problem = MinVar(assets=3, factors=2).build() + problem.update(**_factor_data()) + value = problem.solve() + + assert problem.weights == pytest.approx(np.full(3, 1.0 / 3.0), abs=1e-6) + assert value == pytest.approx(np.sqrt(2.0 + 0.1**2 / 3.0), abs=1e-6) + + +def test_a_rejected_payload_writes_nothing(): + """Validation runs over every model before the first value is written. + + Otherwise a payload rejected on the last model would leave the problem + half-overwritten -- neither the old dataset nor the new one. + """ + problem = MinVar(assets=4).build() + problem.update(**_data(0.0)) + before = problem.solve() + + with pytest.raises(CvxDataError): + problem.update(**(_data(0.9) | {D.UPPER_BOUND_ASSETS: np.ones(4)})) + + assert problem.solve() == pytest.approx(before) + + +def test_a_universe_larger_than_the_compiled_one_is_rejected(): + """Padding only ever goes up, so oversized data is a CvxDataError. + + The models agree with each other here -- they agree on a universe the + compiled problem has no room for -- so this is the `fill` guard rather than + the cross-model one, reached through the public entry point. + """ + problem = MinVar(assets=2).build() + payload = { + D.CHOLESKY: np.eye(4), + D.VOLA_UNCERTAINTY: np.zeros(4), + D.LOWER_BOUND_ASSETS: np.zeros(4), + D.UPPER_BOUND_ASSETS: np.ones(4), + } + + with pytest.raises(CvxDataError, match="does not fit a problem built for"): + problem.update(**payload) diff --git a/tests/test_markowitz/test_utils/test_aux.py b/tests/test_markowitz/test_utils/test_aux.py index b1e1c5e61..f044fb4e4 100644 --- a/tests/test_markowitz/test_utils/test_aux.py +++ b/tests/test_markowitz/test_utils/test_aux.py @@ -1,21 +1,64 @@ -"""Tests for fill utilities that pad vectors/matrices with zeros. +"""Tests for the fill utilities that pad vectors/matrices with zeros. -These tests verify that the helper functions keep existing entries and fill -remaining slots with zeros as required. +The padding is what lets one compiled problem serve a universe smaller than the +one it was built for, so these cover both directions: values kept and the tail +zeroed on the way up, and a `CvxDataError` -- never a silent truncation -- for +input that does not fit at all. """ import numpy as np +import pytest +from cvxmarkowitz import CvxDataError from cvxmarkowitz.utils.fill import fill_matrix, fill_vector def test_fill_vector(): """fill_vector should retain provided values and zero-pad to requested length.""" a = np.ones(2) - np.allclose(fill_vector(num=3, x=a), np.array([1, 1, 0])) + assert fill_vector(num=3, x=a) == pytest.approx(np.array([1.0, 1.0, 0.0])) def test_fill_matrix(): """fill_matrix should embed the input block in the top-left and zero-fill the rest.""" a = np.ones((2, 2)) - np.allclose(fill_matrix(rows=3, cols=3, x=a), np.array([[1, 1, 0], [1, 1, 0], [0, 0, 0]])) + expected = np.array([[1.0, 1.0, 0.0], [1.0, 1.0, 0.0], [0.0, 0.0, 0.0]]) + assert fill_matrix(rows=3, cols=3, x=a) == pytest.approx(expected) + + +def test_fill_vector_exact_fit(): + """A vector of exactly the target length is returned unchanged.""" + a = np.array([1.0, 2.0]) + assert fill_vector(num=2, x=a) == pytest.approx(a) + + +def test_fill_matrix_exact_fit(): + """A matrix of exactly the target shape is returned unchanged.""" + a = np.array([[1.0, 2.0], [3.0, 4.0]]) + assert fill_matrix(rows=2, cols=2, x=a) == pytest.approx(a) + + +def test_fill_vector_too_long(): + """A vector longer than the target raises CvxDataError, not a bare ValueError. + + The compiled problem has no room for the extra entries, and truncating them + would silently drop assets the caller asked about. numpy's own message + ("could not broadcast input array") names neither the caller's mistake nor a + class inside the CvxError tree the README promises. + """ + with pytest.raises(CvxDataError, match="length 3 does not fit a problem built for 2"): + fill_vector(num=2, x=np.ones(3)) + + +@pytest.mark.parametrize( + ("rows", "cols"), + [ + (2, 3), # too many rows + (3, 2), # too many columns + (2, 2), # too many of both + ], +) +def test_fill_matrix_too_large(rows, cols): + """A matrix exceeding the target in either axis raises CvxDataError.""" + with pytest.raises(CvxDataError, match="does not fit a problem built for"): + fill_matrix(rows=rows, cols=cols, x=np.ones((3, 3)))