Repository navigation
Conversation
49f7e21 to
1839466
Compare
2aa2b08 to
e7468e3
Compare
1839466 to
fcb519b
Compare
e7468e3 to
b086056
Compare
4a79cc4 to
d343a85
Compare
7d1003f to
381c9cd
Compare
d343a85 to
2302dcf
Compare
381c9cd to
d2cd90d
Compare
e680eb5 to
cb1db1d
Compare
cb1db1d to
f6d6d51
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): 3 nit.
| """ | ||
| super().train(mode) | ||
| if self.freeze_backbone: | ||
| if isinstance(self.net, nn.ModuleList): |
There was a problem hiding this comment.
nit (simplification): nn.Module.eval() recurses, so self.net.eval() already handles the ModuleList case. The isinstance branch can go: if self.freeze_backbone: self.net.eval().
| if callable(optimizer): | ||
| optimizer = optimizer() | ||
| scale = float(cfg.model.get("backbone_lr_scale", 1.0)) | ||
| groups = None |
There was a problem hiding this comment.
nit (simplification): groups is None exactly when scale == 1.0, and the two branches build the optimizer differently (instantiate with params plus a callable check, versus _partial_=True then a call). Use one path: factory = hydra.utils.instantiate(cfg.model.optimizer, _partial_=True), params = self._backbone_param_groups(lr, scale) if scale != 1.0 else self.trainer.model.parameters(), optimizer = factory(params=params).
|
|
||
|
|
||
| def test_resnet_frozen_backbone_batchnorm_stays_in_eval(): | ||
| """0.15: a frozen ResNet must not update / use batch statistics.""" |
There was a problem hiding this comment.
nit (comments): Drop the "0.15:" prefix: it refers to an internal ideas list.
… LR group Two defects that would confound any backbone comparison: * a frozen `ResNet` backbone kept its BatchNorm layers in train mode, so it normalized with batch statistics and kept updating its running stats. `requires_grad = False` does not touch either. `train()` now puts a frozen backbone in eval mode. Latent today -- no shipped config freezes the ResNet -- so no run to date was affected. * the pretrained backbone and the randomly initialised trunk shared one LR. The new `model.backbone_lr_scale` key gives the encoders' backbones (`backbone_parameters()`) their own param group at `lr * scale`; at the default 1.0 the optimizer is built exactly as before, as a single group over `trainer.model.parameters()`. Nothing shipped sets it to anything else, so the two-group path is exercised only by its tests for now. 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
f6d6d51 to
63e3374
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
|

Fixes two defects that would confound any backbone comparison: a frozen
ResNetbackbone still ran BatchNorm in train mode, and the pretrained backbone had to share one LR with the randomly initialised trunk.Behaviour changes
None for shipped HPT configs. No HPT config freezes the ResNet, so no run to date hit the BatchNorm bug, and
backbone_lr_scaledefaults to 1.0, which builds the optimizer exactly as before.What changed
requires_grad = Falseleaves BatchNorm in train mode, so a "frozen" backbone normalized with batch statistics and kept updating its running stats.ResNet.train()now puts a frozen backbone back in eval mode.model.backbone_lr_scalekey puts the encoders' backbone parameters (backbone_parameters()) in their own param group atlr * scale.trainer.model.parameters(), as before.model.optimizer.lrand raises without one.backbone_parameters()stay in the main group.hpt_bc_mecka_6d_300M.yamldeclaresbackbone_lr_scale: 1.0.ResNetdeclares_hpt_pretrained_attrs = ("net",), the same marker the text stems use, so a selective-init pass can find its pretrained weights. Nothing on this branch reads the marker yet; later branches in the stack do.Usage
The RDT recipe (#680) is the first to use the two-group path:
rdt_bc_mecka_6d_dinov3_vitb.yamlsetsbackbone_lr_scale: 0.1.Tests
New
tests/unit/test_vision_backbone.py: ResNet forward against a reference, BatchNorm mode and running stats under a frozen backbone,backbone_parameters(), the single-group default, the two-group split, frozen-parameter handling, and the base model config'sbackbone_lr_scale: 1.0.🤖 Generated with Claude Code
https://claude.ai/code/session_01HjCQBqQCr2igFEMR7yUovw