Skip to content

fix(td3): allocate K-wide gsp_label buffer for TD3 dtraj E2E path - #32

Open
jdbloom wants to merge 1 commit into
mainfrom
fix/td3-dtraj-gsp-label-buffer
Open

fix(td3): allocate K-wide gsp_label buffer for TD3 dtraj E2E path#32
jdbloom wants to merge 1 commit into
mainfrom
fix/td3-dtraj-gsp-label-buffer

Conversation

@jdbloom

@jdbloom jdbloom commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Bug (reproduced 2026-07-08, job 2478)

A TD3 + GSP_PREDICTION_TARGET=delta_theta_traj + GSP_PREDICTION_HORIZON=5 + GSP_E2E_ENABLED=true cell crashed at ep0:

File ".../GSP-RL/gsp_rl/src/buffers/replay.py", line 141, in store_transition
    self.gsp_label_memory[mem_index] = ( ... )
ValueError: could not broadcast input array from shape (5,) into shape (1,)

Root cause

The K-step trajectory label is shape (K,) = (5,), but the TD3 path's ReplayBuffer was constructed without gsp_label_size, so it defaulted to the scalar width (1,). The DDQN/DQN branches already derived the label width from the K-wide head output (gsp_network_output); the TD3 branch (added in #27/#28) never got the same wiring.

Fix

One-line-of-logic change in the TD3 branch of Actor.build_networks (gsp_rl/src/actors/actor.py), mirroring the DDQN branch verbatim:

gsp_label_sz = int(self.gsp_network_output) if self.gsp_e2e_enabled else 1
self.networks['replay'] = ReplayBuffer(..., gsp_label_size=gsp_label_sz)

gsp_network_output is the config-resolved head width: GSP_PREDICTION_TARGET=delta_theta_traj + GSP_OUTPUT_KIND=delta_theta_traj → size == GSP_PREDICTION_HORIZON (K). No new mechanism — reuses the exact derivation the DDQN path already uses.

Unchanged paths

  • DDQN/DQN branches: untouched (diff is confined to the TD3 branch).
  • TD3 scalar / non-e2e: gsp_label_sz resolves to 1, which is the ReplayBuffer default — buffer allocation is byte-for-byte identical to prior runs.

Tests

Added two build_networks regression tests (config-driven, reproducing the real crash path):

  • test_build_networks_TD3_e2e_dtraj_allocates_K_wide_label_buffer — K=5: asserts gsp_network_output==5, gsp_label_size==5, and that a shape-(5,) label round-trips through store_transition without the broadcast error. Fails on the pre-fix buffer (asserts 1 == 5).
  • test_build_networks_TD3_e2e_scalar_label_buffer_unchanged — K=1: pins the width-1 legacy allocation and scalar round-trip.

Full GSP-RL suite: 570 passed, 4 deselected, coverage 80.9% (≥65% gate).

🤖 Generated with Claude Code

The TD3 E2E ReplayBuffer was constructed without gsp_label_size, so it
defaulted to width 1. With GSP_PREDICTION_TARGET=delta_theta_traj +
GSP_PREDICTION_HORIZON=5 the head predicts a size-K (K=5) trajectory
vector, and storing that shape-(5,) label into the width-1 column crashed
at ep0 (job 2478):

    ValueError: could not broadcast input array from shape (5,) into shape (1,)

The DDQN/DQN branches already derived gsp_label_size from gsp_network_output
(the K-wide head output). Apply the identical wiring to the TD3 branch:
gsp_label_sz = int(self.gsp_network_output) if self.gsp_e2e_enabled else 1.
Scalar targets (K=1) resolve to width 1 == the ReplayBuffer default, so the
legacy TD3 e2e and non-e2e paths are byte-for-byte unchanged. DDQN untouched.

Adds two build_networks regression tests: a config-driven K=5 dtraj case that
reproduces the store_transition broadcast crash and asserts the K-wide label
round-trips, and a K=1 case pinning the width-1 legacy allocation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant