Repository navigation
Conversation
ed3d274 to
ff9dc58
Compare
ff9dc58 to
c01059b
Compare
c01059b to
5f2c373
Compare
e51b3b4 to
3488d12
Compare
3488d12 to
ece3cd2
Compare
5f2c373 to
0b82eaa
Compare
ece3cd2 to
025517c
Compare
0b82eaa to
b01fe95
Compare
025517c to
816e441
Compare
b01fe95 to
7677f76
Compare
816e441 to
b28a7ff
Compare
7677f76 to
648a2a7
Compare
RyanPCo
left a comment
There was a problem hiding this comment.
Automated adversarial review (read-only; findings were checked by a second skeptical pass before posting): 2 low, 1 nit.
| ) | ||
|
|
||
| @property | ||
| def n_tokens(self) -> int: |
There was a problem hiding this comment.
low (dead-code): Nothing reads RDTMemory.n_tokens, and each head's n_memory_tokens has to be set by hand to match trunk.memory.frames. Either derive the heads' n_memory_tokens from it (a hydra interpolation, or set it while building the heads) or delete the property.
| # A history window, (K, H, W, 3): decode each | ||
| # distinct frame once (front padding repeats one). | ||
| window = windows[k][1] | ||
| unique = { |
There was a problem hiding this comment.
low (efficiency): This comprehension still decodes every entry. Front-padding duplicates are decoded and then overwritten, so the comment's "decode each distinct frame once" is not true. Dedupe before decoding: by_idx = dict(zip(map(int, window), data[k])), then unique = {i: simplejpeg.decode_jpeg(b, colorspace="RGB") for i, b in by_idx.items()}.
| # distinct frame once (front padding repeats one). | ||
| window = windows[k][1] | ||
| unique = { | ||
| int(i): simplejpeg.decode_jpeg(b, colorspace="RGB") |
There was a problem hiding this comment.
nit (efficiency): The comment says each distinct frame is decoded once, but the dict comprehension calls simplejpeg.decode_jpeg for every window entry and keeps only the last result per index. Front-padded repeats are decoded K times. Check int(i) not in unique before decoding.
…ide in seconds Human.get_keymap(image_memory=N, image_memory_stride_s=s) adds <front>_mem: N front frames s seconds apart, the newest s seconds back, plus a (N,) <front>_mem_mask of the steps inside the episode. history_stride_s spaces proprio history in seconds, so windows span the same time at any fps. History windows are now point reads instead of a contiguous span thinned afterwards: on JPEG keys (one frame per chunk) a span would fetch every frame between steps. plan_reads still merges them on numeric keys. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UL4UaiqSDNbhpatNwdvJbe
…-step eval MSE _stem_history_len called .get on the policy's stems, an nn.ModuleDict once built, so every proprio-history (K > 1) run crashed at its first batch. The unit tests used a plain dict. The last-step MSE handed torchmetrics a strided [:, -1] slice, which it .view(-1)s; on GPU .cpu() copied it contiguous, on a CPU run it raised. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UL4UaiqSDNbhpatNwdvJbe
RDTMemory: N past frames of one camera through a frozen DINOv3 tower (only its proj trains), a Perceiver resampler shared across frames pooling each to M latents, a sin-cos code of each frame's age in seconds, and optionally that step's proprio token (fuse_proprio). The N x M tokens join the DiT's self-attention prefix [t; freq; state; memory; actions] under a key mask, so steps before the episode start and dropped memories (trunk.memory.dropout) are never attended. Without trunk.memory the model is unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw
Follows the dual-encoder design of diffusion-policy-experiments: - the memory tower is a second DINOv3, fine-tuned at the backbone LR scale (the param-group walk now covers every module with backbone_parameters), pooling each frame to its CLS token; the Perceiver resampler is gone - the window ends at the current frame (t-9 s .. t) - short_dropout (0.3 in the memory arms) replaces a sample's short image stream by a learned null token, replacing whole-memory dropout Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UL4UaiqSDNbhpatNwdvJbe
Backprop through 320 frames / GPU OOMed an H100; with the tower's blocks recomputed the both arm trains at 1.19 s / step on 8xH100. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01UL4UaiqSDNbhpatNwdvJbe
b28a7ff to
36f7cf1
Compare
648a2a7 to
483e60f
Compare
|
Review findings: status after the fixup pass. The fixes are squashed into this PR's commits, and the whole stack was force-pushed. Fixed
|

Adds an optional long-range visual memory to RDT, and the loader support it needs. Without
trunk.memoryandkey_map.image_memory, models and keymaps are unchanged.Loader
Human.get_keymap(image_memory=N, image_memory_stride_s=s)adds<front>_mem: N front frames s seconds apart, ending at the current frame, plus a(N,)<front>_mem_maskof the steps inside the episode. Steps before the episode start repeat the oldest real frame and are masked.history_stride_sspaces proprio history in seconds, read in each episode's fps, so a window spans the same time at any fps. It replaceshistory_stridewhen set.plan_readsstill merges them on numeric keys. Each distinct frame is read and decoded once.Model
RDTMemoryencodes each memory frame to one token with a second DINOv3 tower (DINOv3Stem(pool="cls")), fine-tuned at the backbone LR scale, and tags it with a sin-cos code of the frame's age in seconds.fuse_proprioadds a projection of that step's proprio token, which needs the proprio history on the same grid.[t; freq; state; memory; actions]under a key mask, so padded steps are never attended.short_dropoutreplaces a sample's whole short image stream by a learned null token during training, so the policy cannot lean on the latest frames alone.DINOv3Stemgainspool(patch/cls) andgrad_checkpointing. Backprop through the fine-tuned memory tower (320 frames per GPU) OOMed an H100 without it.backbone_parameters, not just the encoders, so the memory tower lands in it.image_memoryin the batch (or the reverse), a frame-count mismatch, or an encoder that gives more than one token per frame raises.Fixes
HPT._stem_history_lencalled.geton the builtnn.ModuleDict, so every proprio-history (K > 1) run crashed at its first batch. The unit tests used a plain dict.[:, -1]slice, which it.view(-1)s. On GPU.cpu()made it contiguous; on a CPU run it raised.The memory ablation train configs (a 2x2 over proprio history and visual memory on the Mecka 20% split) and a cluster launcher are kept out of this PR.
Tests
tests/unit/test_image_memory.py: frame spacing in seconds, front padding and masking, the first frame, one-frame windows, one read per distinct frame, no memory by default, norm mode dropping the memory camera.tests/unit/test_rdt.py: masked memory tokens ignored, age tagging, short dropout train-only, loss reaching every trainable parameter (with and withoutfuse_proprio), the memory tower in the backbone LR group, and the config/batch agreement checks.tests/unit/test_proprio_history.py: stride in seconds following the episode fps, andhistory_lenread from a builtModuleDict.🤖 Generated with Claude Code
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw