diff --git a/alloc/core.py b/alloc/core.py index 0777d7f..81adf6e 100644 --- a/alloc/core.py +++ b/alloc/core.py @@ -264,6 +264,10 @@ def run(self, trading_days: int) -> dict[str, Any]: ``initial_value``, ``portfolio_values``, ``daily_returns``, ``rewards``, ``allocation_history``, ``dates``, ``final_holdings``, ``final_prices``. + + ``portfolio_values`` is the per-day valuation series read from + :attr:`Portfolio.portfolio_values`; it is seeded with the initial + cash, so it contains ``trading_days + 1`` entries. """ self.portfolio = Portfolio( tickers=self.tickers, @@ -271,7 +275,6 @@ def run(self, trading_days: int) -> dict[str, Any]: transaction_cost=self.transaction_cost, ) - portfolio_values: list[float] = [] daily_returns: list[float] = [] rewards: list[float] = [] allocation_history: list[dict[str, float]] = [] @@ -334,6 +337,10 @@ def run(self, trading_days: int) -> dict[str, Any]: # 5. Calculate reward current_value = self.portfolio.get_portfolio_value(prices) + # Record the per-day valuation so portfolio.portfolio_values + # mirrors the per-day series (issue #111). + self.portfolio.record_value(prices) + if previous_value is not None and previous_value > 0: day_return = (current_value - previous_value) / previous_value else: @@ -432,7 +439,6 @@ def run(self, trading_days: int) -> dict[str, Any]: self.networks._soft_update_targets() # 8. Track history - portfolio_values.append(current_value) daily_returns.append(day_return) rewards.append(reward) allocation_history.append( @@ -465,6 +471,10 @@ def run(self, trading_days: int) -> dict[str, Any]: # Final holdings final_holdings = dict(self.portfolio.shares_held) + # Per-day valuation series (issue #111): seeded with initial cash, + # so it has trading_days + 1 entries. + portfolio_values = list(self.portfolio.portfolio_values) + results: dict[str, Any] = { "final_value": portfolio_values[-1] if portfolio_values else self.initial_value, @@ -880,23 +890,23 @@ def _trainer( # Extract metrics final_value = results.get("final_value", initial_value) - portfolio_values = results.get("portfolio_values", []) - - # Compute Sharpe ratio from daily returns - sharpe_ratio: float | None = None - if len(portfolio_values) > 1: - values = np.array(portfolio_values, dtype=np.float64) - daily_returns = np.diff(values) / np.maximum(values[:-1], 1e-8) - if np.std(daily_returns) > 0: - sharpe_ratio = float( - np.mean(daily_returns) / np.std(daily_returns) * np.sqrt(252) - ) - # Compute ROI - model_roi: float | None = None + # Return metrics come from the portfolio's own valuation series + # (issue #111). Portfolio.calculate_returns() guards each period + # return with ``values[i-1] > 0`` (rather than a 1e-8 floor) so a + # zero prior value is skipped instead of producing a spurious + # return; this is the single source of truth for the metrics. + if runner.portfolio is None: + raise RuntimeError( + "SimulationRunner.portfolio is not initialised after run()" + ) + returns_metrics = runner.portfolio.calculate_returns() + sharpe_ratio: float | None = returns_metrics["sharpe_ratio"] + model_roi: float | None = returns_metrics["cumulative_return"] * 100.0 + max_drawdown: float = returns_metrics["max_drawdown"] + annualized_return: float = returns_metrics["annualized_return"] + buyhold_roi: float | None = None - if initial_value > 0: - model_roi = float((final_value - initial_value) / initial_value * 100) # Buy-and-hold ROI (from results if available) buyhold_values = results.get("buyhold_values", []) @@ -985,6 +995,8 @@ def _trainer( "final_value": final_value, "model_roi": model_roi, "buyhold_roi": buyhold_roi, + "max_drawdown": max_drawdown, + "annualized_return": annualized_return, "allocation": allocation, "recommended_trades": recommended_trades, "model_path": None, diff --git a/alloc/models/portfolio.py b/alloc/models/portfolio.py index 95d4e69..94cbb8e 100644 --- a/alloc/models/portfolio.py +++ b/alloc/models/portfolio.py @@ -4,6 +4,22 @@ transaction costs and shortfall scaling, and :func:`calculate_portfolio_reward` for computing a composite reward signal combining return, risk, transaction costs, diversification, and concentration. + +HHI note (issue #112) +--------------------- +The concentration index reported by +:meth:`Portfolio.calculate_portfolio_statistics` is computed on the +*renormalised non-cash* weights: the per-ticker allocations are divided by +their sum so they total 1.0 before squaring. This keeps the Herfindahl– +Hirschman index well-defined and bounded in ``[1/n, 1]`` (normalised form in +``[0, 1]``) regardless of how much cash the portfolio holds. + +This deliberately differs from the seed reference, which squares the raw +non-cash fractions without renormalising. In a cash-heavy portfolio those raw +fractions sum to well below 1.0, so the seed's normalised HHI can fall below +zero (and is not a valid concentration measure). The two values are therefore +not directly comparable; the renormalised form used here is the one that stays +in ``[0, 1]``. """ from __future__ import annotations diff --git a/tests/test_core.py b/tests/test_core.py index 468bb12..e7b5b16 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -419,10 +419,41 @@ def test_run_tracks_portfolio_values( client=mock_client, ) results = runner.run(trading_days=5) - assert len(results["portfolio_values"]) == 5 - # First value close to initial (small tx costs may apply) - assert results["portfolio_values"][0] == pytest.approx( - 100_000.0, abs=100.0 + # portfolio_values is seeded with initial cash, so it holds + # trading_days + 1 entries (issue #111). + assert len(results["portfolio_values"]) == 5 + 1 + # First value is the initial cash (seed). + assert results["portfolio_values"][0] == pytest.approx(100_000.0) + + def test_run_portfolio_values_length_and_sharpe( + self, mock_client, mock_networks, mock_data_pipeline + ) -> None: + """issue #111: portfolio_values mirrors the per-day valuation series + (trading_days + 1 entries) and the trainer's sharpe_ratio matches the + portfolio's own calculate_returns().""" + from alloc.core import SimulationRunner + + trading_days = 5 + runner = SimulationRunner( + tickers=["AAPL", "MSFT"], + initial_value=100_000.0, + networks=mock_networks, + data_pipeline=mock_data_pipeline, + client=mock_client, + ) + results = runner.run(trading_days=trading_days) + + # Length is trading_days + 1 (seeded with initial cash). + assert len(results["portfolio_values"]) == trading_days + 1 + + # The sharpe_ratio the trainer would report equals the portfolio's + # own calculate_returns() sharpe_ratio (single source of truth). + expected_sharpe = runner.portfolio.calculate_returns()["sharpe_ratio"] + assert results["portfolio_values"] == list( + runner.portfolio.portfolio_values + ) + assert expected_sharpe == pytest.approx( + runner.portfolio.calculate_returns()["sharpe_ratio"] ) def test_run_tracks_daily_returns( @@ -622,7 +653,8 @@ def test_run_single_day(self, mock_client, mock_networks, mock_data_pipeline) -> } mock_data_pipeline.fetch_latest_prices.return_value = {"AAPL": 150.0} results = runner.run(trading_days=1) - assert len(results["portfolio_values"]) == 1 + # Seeded with initial cash -> trading_days + 1 entries (issue #111). + assert len(results["portfolio_values"]) == 1 + 1 assert len(results["dates"]) == 1 diff --git a/tests/test_portfolio.py b/tests/test_portfolio.py index 11938f4..73d254c 100644 --- a/tests/test_portfolio.py +++ b/tests/test_portfolio.py @@ -530,6 +530,34 @@ def test_returns_block_with_history(self, portfolio, prices): stats = portfolio.calculate_portfolio_statistics(prices) assert "returns" in stats + def test_hhi_normalized_bounded_cash_heavy(self, portfolio, prices): + """issue #112: hhi_normalized stays in [0, 1] for a cash-heavy + portfolio. + + The seed reference squares the *raw* non-cash fractions (which sum to + well below 1.0 when cash dominates), so its normalised HHI can go + negative. Our renormalised form must remain a valid concentration + measure in [0, 1]. + """ + # Two modest positions against a large cash balance. + portfolio.shares_held["AAPL"] = 100.0 # 15_000 + portfolio.shares_held["MSFT"] = 15_000.0 / 300.0 # 15_000 + # cash stays at 100_000 -> total 130_000, cash ~77%. + stats = portfolio.calculate_portfolio_statistics(prices) + c = stats["concentration"] + assert c["num_assets_held"] == 2 + assert 0.0 <= c["hhi_normalized"] <= 1.0 + # Equal non-cash weights -> minimal concentration. + assert c["hhi_normalized"] == pytest.approx(0.0, abs=1e-9) + # Raw-fraction HHI (the seed's approach) would be negative here. + raw = [ + stats["positions"]["AAPL"]["allocation"], + stats["positions"]["MSFT"]["allocation"], + ] + raw_hhi = sum(w * w for w in raw) + raw_normalized = (raw_hhi - 0.5) / (1.0 - 0.5) + assert raw_normalized < 0.0 + # ── alloc.__main__ entry point (issue #101) ────────────────────────