fix: migrate ReplayBuffer to np.random.Generator and DEBUG logging

alloc/models/networks.py

@@ -36,6 +36,9 @@ class ReplayBuffer:
     ----------
     capacity : int
         Maximum number of transitions to retain.
+    rng : np.random.Generator, optional
+        NumPy random generator for reproducible sampling.
+        Defaults to ``np.random.default_rng()``.
 
     Example
     -------
@@ -44,14 +47,28 @@ class ReplayBuffer:
     >>> states, actions, rewards, next_states = buf.sample(batch_size=64)
     """
 
-    def __init__(self, capacity: int) -> None:
-        """Initialise the buffer with *capacity* slots."""
+    def __init__(
+        self, capacity: int, rng: np.random.Generator | None = None
+    ) -> None:
+        """Initialise the buffer with *capacity* slots.
+
+        Parameters
+        ----------
+        capacity : int
+            Maximum number of transitions to retain.
+        rng : np.random.Generator, optional
+            NumPy random generator for reproducible sampling.
+            Defaults to ``np.random.default_rng()``.
+        """
         if capacity <= 0:
             raise ValueError(f"capacity must be > 0, got {capacity}")
         self._buffer: deque[
             tuple[np.ndarray, np.ndarray, float, np.ndarray]
         ] = deque(maxlen=capacity)
         self._capacity = capacity
+        self.rng: np.random.Generator = (
+            rng if rng is not None else np.random.default_rng()
+        )
         logger.info("ReplayBuffer initialised with capacity=%d", capacity)
 
     def add(
@@ -74,6 +91,11 @@ def add(
         next_state : np.ndarray
             Observation at time *t+1*.
         """
+        if len(self._buffer) == self._capacity:
+            logger.debug(
+                "ReplayBuffer full (capacity=%d), overwriting oldest entry",
+                self._capacity,
+            )
         self._buffer.append((state, action, float(reward), next_state))
 
     def sample(
@@ -107,7 +129,7 @@ def sample(
                 f"batch_size={batch_size} exceeds buffer length={len(self)}"
             )
 
-        indices = np.random.choice(len(self), size=batch_size, replace=False)
+        indices = self.rng.choice(len(self), size=batch_size, replace=False)
 
         states = np.stack([self._buffer[i][0] for i in indices])
         actions = np.stack([self._buffer[i][1] for i in indices])

tests/test_replay_buffer.py

@@ -149,3 +149,110 @@ def test_len_capped_at_capacity(self) -> None:
         for _ in range(20):
             buf.add(np.array([0.0]), np.array([0.0]), 0.0, np.array([0.0]))
         assert len(buf) == 5
+
+
+class TestReplayBufferRng:
+    """Tests for TICKET-030: np.random.Generator integration."""
+
+    def test_default_rng_is_generator(self) -> None:
+        """Default rng should be a np.random.Generator instance."""
+        buf = ReplayBuffer(capacity=10)
+        assert isinstance(buf.rng, np.random.Generator)
+
+    def test_custom_rng_is_preserved(self) -> None:
+        """A user-provided rng should be stored as-is."""
+        custom_rng = np.random.default_rng(seed=42)
+        buf = ReplayBuffer(capacity=10, rng=custom_rng)
+        assert buf.rng is custom_rng
+
+    def test_reproducible_sampling_with_seed(self) -> None:
+        """Two buffers with the same seed should produce identical samples."""
+        rng = np.random.default_rng(seed=123)
+        buf1 = ReplayBuffer(capacity=100, rng=rng)
+        rng = np.random.default_rng(seed=123)
+        buf2 = ReplayBuffer(capacity=100, rng=rng)
+
+        for i in range(20):
+            buf1.add(
+                np.array([float(i)]),
+                np.array([float(i)]),
+                float(i),
+                np.array([float(i + 1)]),
+            )
+            buf2.add(
+                np.array([float(i)]),
+                np.array([float(i)]),
+                float(i),
+                np.array([float(i + 1)]),
+            )
+
+        s1, a1, r1, ns1 = buf1.sample(batch_size=5)
+        s2, a2, r2, ns2 = buf2.sample(batch_size=5)
+
+        np.testing.assert_array_equal(r1, r2)
+
+    def test_sample_uses_rng_choice_not_np_random(self) -> None:
+        """sample() must use self.rng.choice, not np.random.choice."""
+        custom_rng = np.random.default_rng(seed=99)
+        buf = ReplayBuffer(capacity=10, rng=custom_rng)
+        for i in range(5):
+            buf.add(
+                np.array([float(i)]),
+                np.array([float(i)]),
+                float(i),
+                np.array([float(i + 1)]),
+            )
+        # Should not raise — self.rng.choice exists and works
+        _, _, rewards, _ = buf.sample(batch_size=3)
+        assert len(rewards) == 3
+
+
+class TestReplayBufferDebugLogging:
+    """Tests for TICKET-031: DEBUG logging when buffer is full."""
+
+    def test_debug_log_when_overwriting(self, caplog) -> None:
+        """A DEBUG message should be emitted when the buffer is full."""
+        import logging
+
+        caplog.set_level(logging.DEBUG)
+        buf = ReplayBuffer(capacity=3)
+        for i in range(3):
+            buf.add(
+                np.array([float(i)]),
+                np.array([float(i)]),
+                float(i),
+                np.array([float(i + 1)]),
+            )
+        # Buffer is now full; next add should trigger debug log
+        buf.add(
+            np.array([99.0]),
+            np.array([99.0]),
+            99.0,
+            np.array([100.0]),
+        )
+        debug_messages = [
+            r for r in caplog.records if r.levelno == logging.DEBUG
+        ]
+        assert len(debug_messages) >= 1
+        assert "overwriting oldest entry" in debug_messages[-1].message
+
+    def test_no_debug_log_before_full(self, caplog) -> None:
+        """No DEBUG overwrite message when buffer is not yet full."""
+        import logging
+
+        caplog.set_level(logging.DEBUG)
+        buf = ReplayBuffer(capacity=10)
+        for i in range(5):
+            buf.add(
+                np.array([float(i)]),
+                np.array([float(i)]),
+                float(i),
+                np.array([float(i + 1)]),
+            )
+        debug_messages = [
+            r
+            for r in caplog.records
+            if r.levelno == logging.DEBUG
+            and "overwriting" in r.message
+        ]
+        assert len(debug_messages) == 0

tickets/TICKET-030.md

@@ -0,0 +1,40 @@
+# TICKET-030: ReplayBuffer.sample uses legacy np.random.choice() - migrate to np.random.Generator
+
+## Evidence
+- alloc/models/networks.py line 110:
+  indices = np.random.choice(len(self), size=batch_size, replace=False)
+- This uses the legacy module-level np.random API, which is stateless and not reproducible via seed control.
+- NumPy 1.17+ introduced np.random.Generator via np.random.default_rng() which provides:
+  - Reproducible, isolated RNG state
+  - Better statistical quality (PCG64 default)
+  - Explicit seeding per-instance
+- Other np.random usage in the same file:
+  - Line 266: np.random.seed(seed) - legacy seeding in ActorCriticNetworks.__init__
+  - Line 485: np.random.normal(...) - legacy call in get_allocation()
+
+## Impact
+- Non-reproducible sampling: np.random.choice() draws from a global RNG state. Any other code path that calls np.random.* between two sample() calls changes the distribution. This breaks reproducibility in RL training loops.
+- Thread-safety: The legacy np.random module is not thread-safe. Concurrent training or evaluation can produce data races.
+- Deprecation trajectory: NumPy recommends migrating to Generator API; the legacy API may be deprecated in future releases.
+- Test flakiness: tests/test_replay_buffer.py tests that rely on sampling order may be subtly affected by global RNG state from other tests.
+
+## Suggestion
+1. Add self._rng = np.random.default_rng() to ReplayBuffer.__init__.
+2. Accept an optional seed parameter in __init__ for external seed control.
+3. Replace line 110: indices = self._rng.choice(len(self), size=batch_size, replace=False)
+4. Update tests/test_replay_buffer.py to verify deterministic sampling with a fixed seed.
+
+## Implementation Plan
+1. Modify ReplayBuffer.__init__ to accept seed: Optional[int] = None and store self._rng = np.random.default_rng(seed)
+2. Replace np.random.choice(...) on line 110 with self._rng.choice(...)
+3. Add test test_sample_deterministic_with_seed to tests/test_replay_buffer.py:
+   - Create two buffers with same seed, same data
+   - Assert sample() returns identical indices
+4. Run pytest tests/test_replay_buffer.py -xvs to verify no regressions
+5. (Out of scope for this ticket but noted) Consider migrating lines 266 and 485 in a follow-up ticket
+
+## Verification
+- pytest tests/test_replay_buffer.py -xvs - all existing tests pass
+- New deterministic seed test passes
+- ruff check alloc/models/networks.py - clean
+- mypy alloc/models/networks.py --ignore-missing-imports - clean

tickets/TICKET-031.md

@@ -0,0 +1,42 @@
+# TICKET-031: ReplayBuffer.add should log at DEBUG level when buffer is full and overwriting
+
+## Evidence
+- alloc/models/networks.py lines 58-78: ReplayBuffer.add() method:
+  def add(self, state, action, reward, next_state) -> None:
+      self._buffer.append((state, action, float(reward), next_state))
+- The underlying collections.deque(maxlen=capacity) silently evicts the oldest entry when len(self._buffer) == self._capacity and a new item is appended.
+- There is NO logging in add() - overwrites are invisible at runtime.
+- __init__ logs at INFO level (line 53): logger.info("ReplayBuffer initialised with capacity=%d", capacity) - this is appropriate for initialization.
+- The audit requirement: when the buffer is full and an overwrite occurs, log at DEBUG level (not INFO, to avoid log spam during training).
+
+## Impact
+- Silent data loss: During RL training, the buffer is typically full after the warmup phase. Every add() call after that evicts the oldest transition, but there is no visibility into this.
+- Debugging difficulty: When investigating training instability or reward distribution shifts, operators cannot tell if the buffer is churning transitions at the expected rate.
+- No INFO-level spam: Logging at INFO would produce one log line per transition (potentially millions during training). DEBUG is the correct level - visible when logging.DEBUG is enabled, silent in production.
+
+## Suggestion
+1. In ReplayBuffer.add(), check if the buffer is already at capacity before appending.
+2. Use a counter-based approach to avoid per-transition spam even at DEBUG level:
+   - Add self._overwrite_count = 0 to __init__
+   - Increment on each overwrite, log every 1000th overwrite at DEBUG level
+3. Log format: "ReplayBuffer full (capacity=%d), %d total overwrites"
+
+## Implementation Plan
+1. Add self._overwrite_count = 0 to ReplayBuffer.__init__ (after line 52)
+2. In ReplayBuffer.add(), before self._buffer.append(...), add:
+   if len(self._buffer) == self._capacity:
+       self._overwrite_count += 1
+       if self._overwrite_count % 1000 == 0:
+           logger.debug("ReplayBuffer full (capacity=%d), %d total overwrites", self._capacity, self._overwrite_count)
+3. Add test test_add_logs_debug_on_overflow to tests/test_replay_buffer.py:
+   - Create buffer with capacity=3
+   - Add 5 transitions
+   - Assert _overwrite_count == 2
+   - Use caplog to verify DEBUG-level log was emitted
+4. Run pytest tests/test_replay_buffer.py -xvs to verify no regressions
+
+## Verification
+- pytest tests/test_replay_buffer.py -xvs - all tests pass
+- New overflow logging test passes with caplog.at_level(logging.DEBUG)
+- ruff check alloc/models/networks.py - clean
+- mypy alloc/models/networks.py --ignore-missing-imports - clean