Repository navigation
Conversation
211db25 to
6efcdae
Compare
43cdc6c to
e10ccbe
Compare
6efcdae to
2fb9f92
Compare
2fb9f92 to
663b77b
Compare
446a560 to
fcdbf6e
Compare
1cb4602 to
5980661
Compare
fcdbf6e to
f1dee44
Compare
5980661 to
5e662b3
Compare
f1dee44 to
d3e5b0d
Compare
5e662b3 to
a4b4fd0
Compare
d3e5b0d to
f20e8aa
Compare
a4b4fd0 to
7e3a333
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 high, 2 nit.
| model_path, | ||
| sum(p.numel() for p in target.parameters()), | ||
| ) | ||
| elif init_source == "paligemma": |
There was a problem hiding this comment.
high (correctness): Any rebuild of a PaliGemma-init PI from its saved config downloads PaliGemma again. paligemma_weight_path is saved in the checkpoint's config_tree, and two paths miss it. trainHydra._build_model_config_tree nulls only pytorch_weight_path on resume/requeue: that key is already null for this arm, so has_weights is False and nothing is nulled. rollout._patch_checkpoint_paths returns early when pytorch_weight_path is None. As a result, every resumed rank and the robot rollout call snapshot_download for the gated google/paligemma-3b-pt-224 and load 3B weights that the checkpoint state_dict then overwrites. Without network access or a Gemma-licensed HF token, this fails. Please null paligemma_weight_path alongside pytorch_weight_path in both places when weights come from a checkpoint, and add a resolve test that sets ckpt_path on the paligemma-init config.
| init_source = select_init_source( | ||
| self.config.pytorch_weight_path, paligemma_weight_path | ||
| ) | ||
| target = ( |
There was a problem hiding this comment.
nit (dead-code): self.model was just built from PI0Pytorch(model_cfg) above, so it can never be DistributedDataParallel here. Since this block is being moved anyway, pass self.model straight to both loaders and drop target.
| pgi.resolve_paligemma_dir(CANONICAL, allow_mirror=True) | ||
|
|
||
|
|
||
| def test_the_mirror_fallback_can_be_switched_off(monkeypatch, tmp_path): |
There was a problem hiding this comment.
nit (test-quality): This duplicates test_the_mirror_is_opt_in, because allow_mirror already defaults to False. test_only_the_shards_are_downloaded (line 305) only re-asserts a module constant; either delete it or assert the allow_patterns the stubbed snapshot_download receives. In test_a_gated_canonical_repo_falls_back_to_a_matching_mirror, good and its digest entry are overwritten by the update on line 349; build digests in one comprehension.
_hf_offline monkeypatched transformers.utils.hub._is_offline_mode, a module flag that 5.x replaced with is_offline_mode(). setattr on a name the module no longer has raises, so every test standing on the harness errored out before it reached the network: 12 of them, including the whole PI train-step matrix. Verified failing identically on an unmodified tree, so this is the venv's transformers 5.17 meeting a 4.x assumption, not a regression from this stack. Probe for whichever spelling the installed version has rather than pinning either, since the lockfile still says 4.53.2. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Xh3wMw9pXYPHUSGjU7qmix
An arm for training pi0.5 from the released VLM instead of the pi0.5 base
checkpoint, to separate what the VLM pretraining buys from what pi0.5's own
robot pretraining buys. The prefix (SigLIP tower, Gemma-2B, the projector)
comes from google/paligemma-3b-pt-224; everything pi0.5 adds on top -- the
Gemma-300M action expert, action_in/out_proj, the adaRMS time MLPs -- stays at
its init.
pytorch_weight_path and paligemma_weight_path are mutually exclusive rather
than ordered by precedence: silently preferring one would make a typo in the
other look like it worked. Setting neither now warns instead of loading
nothing quietly. The loader streams shard by shard and then fails if any
parameter went untouched or any checkpoint tensor went unused, because a
partial load of a 3.6B model is invisible in the loss for a long time.
Two places where the release does not line up with what openpi builds, both
handled in the key remap:
- transformers 5 flattened SiglipVisionModel, so vision_tower.vision_model.*
is vision_tower.* there; below 5 (the pin, 4.53.2) the hop is still in the
module tree. The loader maps onto whichever the live model has. Mapping to
the 5.x spelling unconditionally failed 21 SigLIP tensors on the pin, which
the key-string tests could not see; a unit test now loads a tiny real
PaliGemmaForConditionalGeneration under the installed transformers, and
passes on both 4.53.2 and 5.17.0.
- The checkpoint pads the vocabulary to 257216, 64 rows past the 257152
openpi allocates (<image> plus 63 unused). embed_tokens and lm_head are
truncated; openpi reaches the image slots through the vision tower, never
through an embed_tokens lookup, so the dropped rows are unreachable.
google/paligemma-3b-pt-224 is license-gated. With paligemma_allow_mirror: true,
resolve_paligemma_dir falls back on a 401/403 (not on a rate limit or an outage)
to ungated mirrors whose shards are sha256-checked against the canonical repo's
LFS metadata first -- a mirror that has drifted is refused, not used. It is off
by default: integrity is checked, but a research arm's weights of record should
come from the canonical repo unless a run opts in, and this cluster's HF token
is authorized for it. Only *.safetensors are pulled. openpi's own loader source
is not an option: its GCS bucket 403s anonymously.
pi0.5_paligemma_init is an overlay, not a runnable config, so a keypoint or 6D
arm is four lines. It changes the init source only; the constant LR stays,
since pi0.5_base documents dropping the cosine schedule on purpose.
openpi_compat carries three shims for a transformers 5 venv, which openpi's pin
(4.53.2, also this repo's) does not expect; all should be deleted once the fork's
transformers_replace patch is ported forward. The third restores openpi's fp32
keep-list, which spells the pre-flattening vision paths, so on 5.x the SigLIP
patch and position embeddings ran in bf16. On 5.x only this arm loads: the pi0.5
base checkpoint stores the nested SigLIP names and its strict load_model stops
at startup. pi05.md says which setup is supported and what differs.
Verified with a full trainHydra run to exit 0 on an H100 (4 epochs, all four
validation heads, checkpoint and videos written) and the real 12 GB checkpoint
into the real model in tests/integration, both on transformers 5.17, plus the
unit tests on 4.53.2 and 5.17.0.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Xh3wMw9pXYPHUSGjU7qmix
f20e8aa to
a519280
Compare
7e3a333 to
3c557cc
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 a pi0.5 arm that starts from the released PaliGemma VLM instead of the pi0.5 base checkpoint. This separates what VLM pretraining buys from what pi0.5's robot pretraining buys.
google/paligemma-3b-pt-224: the SigLIP tower, Gemma-2B and the projector.action_in/out_projand the adaRMS time MLPs.Usage
model/pi0.5_paligemma_init.yamlis an overlay. Compose it after anypi0.5_*model config:pi0.5_base's.$EGOVERSE_PALIGEMMA_WEIGHTSpoints it at a local snapshot.pi05.md).Loader
pytorch_weight_pathandpaligemma_weight_pathare mutually exclusive. Setting both is an error; setting neither warns.vision_model.level; on 5.x it is flattened. An earlier version always mapped to the 5.x names and failed 21 tensors on the pin. The key-string unit tests couldn't see that, so a test now loads a tiny realPaliGemmaForConditionalGeneration; it passes on 4.53.2 and 5.17.0.embed_tokensandlm_headare truncated to openpi's 257152. The dropped rows are<image>plus 63 padding tokens, and openpi never looks them up.Gated repo and mirrors
google/paligemma-3b-pt-224is license-gated. Downloading it needs an HF token that has accepted the license.paligemma_allow_mirror: true, a 401/403 on the canonical repo falls back to third-party mirrors. Rate limits and outages don't trigger it. Every mirror shard is sha256-checked against the canonical repo's metadata before use.*.safetensorsare downloaded.transformers 4.53 vs 5.x
transformers_replacepatch.openpi_compat.pyshims for a 5.x venv (all to delete once the fork's patch is ported forward):get_image_featuresreturns an output object, not a tensor.language_model/vision_tower/multi_modal_projectorforwarders are gone.pi05.mddocuments all of this.Verification
trainHydrarun to exit 0 on an H100 (4 epochs, all four validation heads, checkpoint and videos written), on transformers 5.17.tests/integration/test_paligemma_init_real.py), on 5.17. It also asserts the SigLIP embeddings stay fp32.🤖 Generated with Claude Code
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw