Repository navigation
Conversation
49f7e21 to
1839466
Compare
1839466 to
fcb519b
Compare
ad65390 to
7d1003f
Compare
0da7a6f to
85910f0
Compare
7d1003f to
381c9cd
Compare
85910f0 to
c7357bc
Compare
d2cd90d to
84bb0db
Compare
84bb0db to
9a3eeac
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): 1 low, 2 nit.
| algo.nets = nn.ModuleDict() | ||
| algo.nets.train(training) | ||
| algo.encoders = {"front_img_1": {}} | ||
| algo.train_image_augs = ( |
There was a problem hiding this comment.
low (test-quality): _hpt_stub wraps train_image_augs in PerSampleAugs itself, so no test checks that HPT.init does the wrapping, which is the actual fix. Move the wrapping into a small helper that both init and the stub call (or test it through the constructor path), and write test_missing_train_augs_pass_through as _hpt_stub(training=True) instead of re-inlining the stub.
| """ | ||
| if self.nets.training and self.train_image_augs and short in self.encoders: | ||
| return self.train_image_augs(images) | ||
| elif self.eval_image_augs and short in self.encoders: |
There was a problem hiding this comment.
nit (clarity): In training with train_image_augs=None, this falls through to eval_image_augs. Before this PR that case raised, and nothing documents the fallback. Restructure so the choice is explicit: if short not in self.encoders: return images, then augs = self.train_image_augs if self.nets.training else self.eval_image_augs, then return augs(images) if augs is not None else images (or keep the eval fallback on purpose and say so). Also make the docstring a one-line summary.
| @@ -0,0 +1,133 @@ | |||
| """Per-sample image augmentation (ideas 0.10). | |||
There was a problem hiding this comment.
nit (comments): Drop "(ideas 0.10)" and the sentence about "the old inline" call, and remove the print() in test_per_sample_augs_jitters_each_image_independently.
stem input_dim, infer_ac_dims, act_dim and enc-dec output_dim now interpolate
robomimic_model.dims.{<emb>.proprio,<emb>.action,action_width}. Resolved values
are unchanged (pinned by tests/unit/test_model_dims.py). Pi configs untouched.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MWtR1x5tjREwoxQZAnUGrh
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01BbQNNaMAbTNMypegmJkv4s
torchvision draws one random parameter set per call, so applying the configured train_image_augs to the stacked (B, 3, H, W) camera batch gave every image in the batch the same jitter. Wrap train_image_augs in a new PerSampleAugs module so each image draws its own parameters, and move the inline aug block of _robomimic_to_hpt_data into _apply_image_augs with the same gating. Deterministic eval augs stay batched. _apply_image_augs also checks train_image_augs before calling it: __init__ accepts None and train mode would have raised TypeError. The loop is not free: measured on an A40 at batch 64 with the shipped ColorJitter + Normalize, 13.8 -> 92.2 ms at 224x224 and 62.2 -> 105.5 ms at the camera's 360x640, per camera per training step. Correctness first here; the cost is removed further up the stack, where the jitter itself is vectorised over the batch. Co-Authored-By: Claude Fable 5.1 <[email protected]> Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01VBmhpVy7ZcMtsBJWh38cJp
9a3eeac to
fd4c50b
Compare
072b971 to
fba6218
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
|

torchvision draws one random parameter set per call, so applying the
configured train_image_augs to the stacked (B, 3, H, W) camera batch gave
every image in the batch the same jitter. Wrap train_image_augs in a new
PerSampleAugs module so each image draws its own parameters, and move the
inline aug block of _robomimic_to_hpt_data into _apply_image_augs with the
same gating. Deterministic eval augs stay batched.
Co-Authored-By: Claude Fable 5.1 [email protected]
🤖 Generated with Claude Code
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw