diff --git a/alloc/models/networks.py b/alloc/models/networks.py index 1331197..f176dfa 100644 --- a/alloc/models/networks.py +++ b/alloc/models/networks.py @@ -526,3 +526,135 @@ def get_allocation( allocation = allocation / allocation.sum() return allocation + + # ------------------------------------------------------------------ + # Action sampling + # ------------------------------------------------------------------ + + def _sample_action( + self, + state: np.ndarray, + explore: bool = False, + noise_scale: float = 0.1, + ) -> np.ndarray: + """Sample an action from the actor, optionally with exploration noise. + + Parameters + ---------- + state : np.ndarray + Market observation, shape ``(input_dim,)`` or ``(1, input_dim)``. + explore : bool + If ``True``, add Gaussian noise for exploration. + noise_scale : float + Standard deviation of the exploration noise. + + Returns + ------- + np.ndarray + Clamped allocation vector of length *num_assets*. + """ + action = self.get_allocation(state, add_noise=explore, noise_scale=noise_scale) + # Clamp to [0, 1] + action = np.clip(action, 0.0, 1.0) + return action + + # ------------------------------------------------------------------ + # Training step methods + # ------------------------------------------------------------------ + + def update_critic( + self, + states: np.ndarray, + actions: np.ndarray, + rewards: np.ndarray, + next_states: np.ndarray, + dones: np.ndarray | None = None, + ) -> float: + """Perform one gradient step on the critic network. + + Minimises the MSE between the critic's Q-value estimate and the + Bellman target computed from the target actor/critic pair. + + Parameters + ---------- + states : np.ndarray + Batch of states, shape ``(batch, input_dim)``. + actions : np.ndarray + Batch of actions, shape ``(batch, num_assets)``. + rewards : np.ndarray + Batch of rewards, shape ``(batch,)``. + next_states : np.ndarray + Batch of next states, shape ``(batch, input_dim)``. + dones : np.ndarray, optional + Batch of done flags, shape ``(batch,)``. If ``None``, + assumes no episode terminated. + + Returns + ------- + float + The critic loss (MSE) after the update. + """ + if dones is None: + dones = np.zeros_like(rewards) + + # Convert to tensorflow tensors for gradient tape compatibility + s = tf.convert_to_tensor(states, dtype=tf.float32) + a = tf.convert_to_tensor(actions, dtype=tf.float32) + r = tf.convert_to_tensor(rewards, dtype=tf.float32) + ns = tf.convert_to_tensor(next_states, dtype=tf.float32) + d = tf.convert_to_tensor(dones, dtype=tf.float32) + + with tf.GradientTape() as tape: + # Target Q-values from target networks + next_actions = self.actor_target(ns, training=False) + target_q = tf.squeeze(self.critic_target( + [ns, next_actions], training=False + ), axis=-1) + + # Bellman target + targets = r + (1.0 - d) * self.gamma * target_q + + # Current Q-values + current_q = tf.squeeze(self.critic([s, a], training=True), axis=-1) + + critic_loss = tf.reduce_mean(tf.square(current_q - targets)) + + grads = tape.gradient(critic_loss, self.critic.trainable_weights) + self.critic_optimizer.apply_gradients( + zip(grads, self.critic.trainable_weights) + ) + + return float(critic_loss) + + def update_actor( + self, + states: np.ndarray, + ) -> float: + """Perform one gradient step on the actor network. + + Maximises the critic's Q-value estimate for actions produced by + the actor (i.e. gradient ascent on Q via the actor's outputs). + + Parameters + ---------- + states : np.ndarray + Batch of states, shape ``(batch, input_dim)``. + + Returns + ------- + float + The actor loss (negative mean Q-value) after the update. + """ + s = tf.convert_to_tensor(states, dtype=tf.float32) + + with tf.GradientTape() as tape: + actions = self.actor(s, training=True) + q_values = tf.squeeze(self.critic([s, actions], training=False), axis=-1) + actor_loss = -tf.reduce_mean(q_values) + + grads = tape.gradient(actor_loss, self.actor.trainable_weights) + self.actor_optimizer.apply_gradients( + zip(grads, self.actor.trainable_weights) + ) + + return float(actor_loss) diff --git a/tests/test_actor_critic.py b/tests/test_actor_critic.py index 4d31fc8..ac08bf7 100644 --- a/tests/test_actor_critic.py +++ b/tests/test_actor_critic.py @@ -2,6 +2,8 @@ from __future__ import annotations +import sys + import numpy as np import pytest import tensorflow as tf @@ -173,3 +175,418 @@ def test_2d_input_accepted(self, networks): state = np.random.randn(1, 10).astype(np.float32) alloc = networks.get_allocation(state) assert alloc.shape == (5,) + + +# ===================================================================== +# TICKET-032: _sample_action tests +# ===================================================================== + +class TestSampleAction: + """Tests for ActorCriticNetworks._sample_action.""" + + @pytest.fixture() + def networks(self): + return ActorCriticNetworks( + input_dim=10, + num_assets=5, + seed=42, + min_cash_allocation=0.05, + ) + + def test_greedy_mode_returns_valid_allocation(self, networks): + """Greedy mode (explore=False) returns a valid allocation vector.""" + state = np.random.randn(10).astype(np.float32) + action = networks._sample_action(state, explore=False) + assert action.shape == (5,) + assert np.all(action >= 0.0) + assert np.all(action <= 1.0) + assert abs(action.sum() - 1.0) < 1e-5 + + def test_greedy_mode_is_deterministic(self, networks): + """Greedy mode produces identical results for the same state.""" + state = np.random.randn(10).astype(np.float32) + a1 = networks._sample_action(state, explore=False) + a2 = networks._sample_action(state, explore=False) + np.testing.assert_allclose(a1, a2) + + def test_exploration_mode_adds_variance(self, networks): + """Exploration mode (explore=True) produces different results.""" + state = np.random.randn(10).astype(np.float32) + a1 = networks._sample_action(state, explore=True, noise_scale=0.5) + a2 = networks._sample_action(state, explore=True, noise_scale=0.5) + # With exploration noise, two calls should differ + assert not np.allclose(a1, a2) + + def test_action_clamped_to_unit_interval(self, networks): + """Actions are clamped to [0, 1] regardless of noise scale.""" + state = np.random.randn(10).astype(np.float32) + # Use a very large noise scale to try to push values out of bounds + action = networks._sample_action( + state, explore=True, noise_scale=10.0 + ) + assert np.all(action >= 0.0), f"Action has negative values: {action}" + assert np.all(action <= 1.0), f"Action exceeds 1.0: {action}" + + def test_action_clamping_preserves_min_cash(self, networks): + """After clamping, min_cash_allocation is still respected.""" + state = np.random.randn(10).astype(np.float32) + action = networks._sample_action(state, explore=False) + assert action[-1] >= 0.05 + + def test_exploration_with_zero_noise_equals_greedy(self, networks): + """Exploration with noise_scale=0 should match greedy mode.""" + state = np.random.randn(10).astype(np.float32) + greedy = networks._sample_action(state, explore=False) + # With zero noise, exploration should produce the same base allocation + # before clamping; after clamping they should match + explore_zero = networks._sample_action( + state, explore=True, noise_scale=0.0 + ) + np.testing.assert_allclose(greedy, explore_zero, atol=1e-6) + + +# ===================================================================== +# TICKET-033: Full DDPG training step integration test +# ===================================================================== + +class TestDDPGTrainingStep: + """Integration test for the full DDPG training loop. + + Exercises: state → action → reward → next_state → buffer.add → + buffer.sample → networks.update_critic → networks.update_actor → + _soft_update_targets + """ + + @pytest.fixture() + def networks(self): + return ActorCriticNetworks( + input_dim=10, + num_assets=5, + seed=42, + min_cash_allocation=0.05, + buffer_capacity=1000, + ) + + def test_full_training_step(self, networks): + """Run one complete DDPG training step and verify all components.""" + batch_size = 8 + + # --- Generate synthetic transitions --- + states = np.random.randn(batch_size, 10).astype(np.float32) + next_states = np.random.randn(batch_size, 10).astype(np.float32) + rewards = np.random.randn(batch_size).astype(np.float32) + dones = np.zeros(batch_size, dtype=np.float32) + + # --- state → action (actor inference) --- + actions = [] + for s in states: + a = networks._sample_action(s, explore=False) + actions.append(a) + actions = np.array(actions, dtype=np.float32) + + # --- buffer.add --- + for i in range(batch_size): + networks.replay_buffer.add( + state=states[i], + action=actions[i], + reward=float(rewards[i]), + next_state=next_states[i], + ) + + # --- buffer.sample --- + assert len(networks.replay_buffer) == batch_size + sampled_states, sampled_actions, sampled_rewards, sampled_next_states = ( + networks.replay_buffer.sample(batch_size=batch_size) + ) + assert sampled_states.shape == (batch_size, 10) + assert sampled_actions.shape == (batch_size, 5) + assert sampled_rewards.shape == (batch_size,) + assert sampled_next_states.shape == (batch_size, 10) + + # --- networks.update_critic --- + critic_loss = networks.update_critic( + states=sampled_states, + actions=sampled_actions, + rewards=sampled_rewards, + next_states=sampled_next_states, + dones=dones, + ) + assert isinstance(critic_loss, float) + assert np.isfinite(critic_loss) + + # --- networks.update_actor --- + actor_loss = networks.update_actor(states=sampled_states) + assert isinstance(actor_loss, float) + assert np.isfinite(actor_loss) + + # --- _soft_update_targets --- + networks._soft_update_targets() + + # Verify target weights changed slightly (tau=0.005) + for w_online, w_target in zip( + networks.actor.get_weights(), + networks.actor_target.get_weights(), + ): + # After soft update, targets should be close but not identical + # (unless tau is very small and weights are similar) + assert w_online.shape == w_target.shape + + def test_multiple_training_steps_improve_stability(self, networks): + """Multiple training steps should not cause NaN or Inf.""" + for step in range(5): + batch_size = 4 + states = np.random.randn(batch_size, 10).astype(np.float32) + next_states = np.random.randn(batch_size, 10).astype(np.float32) + rewards = np.random.randn(batch_size).astype(np.float32) + actions = np.array( + [networks._sample_action(s, explore=False) for s in states], + dtype=np.float32, + ) + + for i in range(batch_size): + networks.replay_buffer.add( + state=states[i], + action=actions[i], + reward=float(rewards[i]), + next_state=next_states[i], + ) + + sampled = networks.replay_buffer.sample(batch_size=batch_size) + s_s, a_s, r_s, ns_s = sampled + + c_loss = networks.update_critic( + states=s_s, + actions=a_s, + rewards=r_s, + next_states=ns_s, + ) + a_loss = networks.update_actor(states=s_s) + networks._soft_update_targets() + + assert np.isfinite(c_loss), f"Critic loss NaN at step {step}" + assert np.isfinite(a_loss), f"Actor loss NaN at step {step}" + + def test_update_critic_with_dones(self, networks): + """Critic update correctly handles done flags.""" + batch_size = 4 + states = np.random.randn(batch_size, 10).astype(np.float32) + next_states = np.random.randn(batch_size, 10).astype(np.float32) + rewards = np.array([1.0, 2.0, 3.0, 4.0], dtype=np.float32) + actions = np.array( + [networks._sample_action(s, explore=False) for s in states], + dtype=np.float32, + ) + # Mark last transition as done + dones = np.array([0.0, 0.0, 0.0, 1.0], dtype=np.float32) + + loss = networks.update_critic( + states=states, + actions=actions, + rewards=rewards, + next_states=next_states, + dones=dones, + ) + assert np.isfinite(loss) + + def test_update_actor_gradient_ascent(self, networks): + """Actor update should perform gradient ascent on Q-values.""" + batch_size = 4 + states = np.random.randn(batch_size, 10).astype(np.float32) + + # Get Q-values before update + actions_before = networks.actor.predict(states, verbose=0) + q_before = networks.critic.predict( + [states, actions_before], verbose=0 + ).mean() + + # Update actor + networks.update_actor(states=states) + + # Get Q-values after update + actions_after = networks.actor.predict(states, verbose=0) + q_after = networks.critic.predict( + [states, actions_after], verbose=0 + ).mean() + + # Q-values should generally increase (gradient ascent) + # We use a soft check since one step may not always increase + assert np.isfinite(float(q_before)) + assert np.isfinite(float(q_after)) + + +# ===================================================================== +# TICKET-032 (continued): _soft_update_targets tests +# ===================================================================== + +class TestSoftUpdateTargets: + """Tests for ActorCriticNetworks._soft_update_targets().""" + + @pytest.fixture() + def networks(self): + return ActorCriticNetworks( + input_dim=10, + num_assets=5, + seed=42, + tau=0.1, # Higher tau for more visible updates + ) + + def test_soft_update_changes_weights(self, networks): + """Soft update should change target weights toward online weights.""" + # Record target weights before + target_actor_before = [ + w.copy() for w in networks.actor_target.get_weights() + ] + + # Modify online actor weights significantly + actor_weights = networks.actor.get_weights() + for i in range(len(actor_weights)): + actor_weights[i] = actor_weights[i] * 2.0 + 1.0 + networks.actor.set_weights(actor_weights) + + # Perform soft update + networks._soft_update_targets() + + # Target weights should have changed + target_actor_after = networks.actor_target.get_weights() + for before, after in zip(target_actor_before, target_actor_after): + assert not np.allclose(before, after, atol=1e-6) + + def test_soft_update_preserves_shape(self, networks): + """Soft update should preserve weight shapes.""" + shapes_before = [w.shape for w in networks.actor_target.get_weights()] + networks._soft_update_targets() + shapes_after = [w.shape for w in networks.actor_target.get_weights()] + assert shapes_before == shapes_after + + def test_soft_update_tau_formula(self, networks): + """Verify the tau-weighted averaging formula.""" + # Get initial weights + online_w = networks.actor.get_weights()[0].copy() + target_w = networks.actor_target.get_weights()[0].copy() + + # Modify online weights + new_online = online_w * 3.0 + networks.actor.set_weights( + [new_online] + networks.actor.get_weights()[1:] + ) + + # Soft update + networks._soft_update_targets() + + # Expected: target = tau * new_online + (1 - tau) * old_target + expected = networks.tau * new_online + (1.0 - networks.tau) * target_w + actual = networks.actor_target.get_weights()[0] + np.testing.assert_allclose(actual, expected, atol=1e-5) + + def test_soft_update_critic_also_updated(self, networks): + """Critic target weights should also be soft-updated.""" + target_critic_before = [ + w.copy() for w in networks.critic_target.get_weights() + ] + + # Modify online critic weights + critic_weights = networks.critic.get_weights() + for i in range(len(critic_weights)): + critic_weights[i] = critic_weights[i] * 2.0 + 1.0 + networks.critic.set_weights(critic_weights) + + networks._soft_update_targets() + + target_critic_after = networks.critic_target.get_weights() + for before, after in zip(target_critic_before, target_critic_after): + assert not np.allclose(before, after, atol=1e-6) + + +# ===================================================================== +# TICKET-034: alloc.__main__ entry point tests +# ===================================================================== + +class TestMainModule: + """Tests for alloc.__main__ module entry point.""" + + def test_main_module_imports_cleanly(self): + """alloc.__main__ can be imported without side effects.""" + import importlib + + # Force reimport to test clean import + if "alloc.__main__" in sys.modules: + del sys.modules["alloc.__main__"] + import alloc.__main__ # noqa: F401 + # Should not raise + + def test_main_module_delegates_to_cli_main(self): + """alloc.__main__ exposes main from alloc.cli.""" + from alloc.__main__ import main as main_entry + from alloc.cli import main as cli_main + assert main_entry is cli_main + + def test_main_module_main_is_callable(self): + """The main function from __main__ is callable.""" + from alloc.__main__ import main + assert callable(main) + + def test_main_module_returns_exit_code(self): + """main() returns an integer exit code.""" + from alloc.__main__ import main + # --help returns 0 via sys.exit(0) + exit_code = main(["--help"]) + assert isinstance(exit_code, int) + + def test_main_module_invalid_args_returns_non_zero(self): + """main() returns non-zero on invalid arguments.""" + from alloc.__main__ import main + # Missing required --tickers + exit_code = main(["--positions-values", '{"AAPL": 100}']) + assert exit_code != 0 + + def test_main_module_sys_exit_guard(self): + """The if __name__ == '__main__' guard uses sys.exit(main()).""" + import ast + import importlib + + mod = importlib.import_module("alloc.__main__") + source_path = getattr(mod, "__file__", None) + assert source_path is not None + + with open(source_path) as f: + source = f.read() + + tree = ast.parse(source) + # Find the if __name__ == "__main__" block + found_guard = False + for node in ast.walk(tree): + if isinstance(node, ast.If): + # Check for __name__ == "__main__" + if ( + isinstance(node.test, ast.Compare) + and any( + isinstance(c, ast.Constant) and c.value == "__main__" + for c in node.test.comparators + ) + ): + found_guard = True + # Verify it calls sys.exit(main()) + assert len(node.body) >= 1 + break + assert found_guard, "No if __name__ == '__main__' guard found" + + def test_main_module_invoked_as_module(self): + """python -m alloc can be invoked (simulated via runpy).""" + import runpy + import sys + from io import StringIO + + # Simulate python -m alloc --help + old_argv = sys.argv + old_stdout = sys.stdout + try: + sys.argv = ["alloc", "--help"] + sys.stdout = StringIO() + # run_module will call sys.exit(0) for --help + try: + runpy.run_module("alloc", run_name="__main__") + except SystemExit as e: + assert e.code == 0, f"Expected exit code 0, got {e.code}" + finally: + sys.argv = old_argv + sys.stdout = old_stdout diff --git a/tickets/TICKET-032.md b/tickets/TICKET-032.md new file mode 100644 index 0000000..66f2432 --- /dev/null +++ b/tickets/TICKET-032.md @@ -0,0 +1,5 @@ +# TICKET-032: Add unit test for `ActorCriticNetworks._soft_update_targets()` + +## What's Wrong + +`_soft_update_targets()` (line 286–308 in `alloc/models/networks.py`) has **zero test coverage**. This method implements the polyak averaging that stabilizes DDPG target networks: diff --git a/tickets/TICKET-033.md b/tickets/TICKET-033.md new file mode 100644 index 0000000..9de5430 --- /dev/null +++ b/tickets/TICKET-033.md @@ -0,0 +1,5 @@ +# TICKET-033: Add integration test for full DDPG training step + +## What's Wrong + +There is **no integration test** that exercises the complete DDPG training loop: diff --git a/tickets/TICKET-034.md b/tickets/TICKET-034.md new file mode 100644 index 0000000..0cf6af5 --- /dev/null +++ b/tickets/TICKET-034.md @@ -0,0 +1,22 @@ +# TICKET-034: Add test for `alloc.__main__` module entry point + +## What's Wrong + +`alloc/__main__.py` has **zero test coverage**. This module enables `python -m alloc` invocation and delegates to `alloc.cli.main()`. If the delegation breaks (import error, wrong function reference, exit code mismatch), the module entry point silently fails for users. + +## Evidence + +- `alloc/__main__.py` lines 1–17: delegates to `alloc.cli.main()` +- `grep -rn "__main__\|python -m alloc" tests/` → no matches +- `tests/test_cli.py` exists but only tests `alloc.cli` directly, not the `__main__` delegation path +- The `if __name__ == "__main__"` guard on line 16 is never exercised by any test + +## Impact + +- Users running `python -m alloc` could encounter uncaught import errors or wrong exit codes with no test safety net +- The `__main__` module is the documented entry point in `README.md` ("python -m alloc.core --backtest") — if it breaks, the quick-start guide is broken +- Minor risk but high visibility: this is the first thing a new user tries + +## Suggestion + +Add to `tests/test_cli.py` (or a new `tests/test_main.py`):