Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
| class _DynamicsStateMixin: | ||
| """Include dynamics parameters in solver checkpoints.""" | ||
|
|
||
| def state_dict(self) -> dict: | ||
| """Save the encoder, optimizer, criterion, and dynamics model.""" | ||
| state = super().state_dict() | ||
| state["dynamics_model"] = self.dynamics_model.state_dict() | ||
| return state | ||
|
|
||
| def load_state_dict(self, state_dict: dict, strict: bool = True): | ||
| """Restore dynamics; reject incomplete legacy checkpoints in strict mode.""" | ||
| if "dynamics_model" not in state_dict: | ||
| message = ( | ||
| "Checkpoint is missing dynamics_model parameters. " | ||
| "Legacy DCL checkpoints did not save them. " | ||
| "Use strict=False only to retain the initialized dynamics.") | ||
| if strict: | ||
| raise KeyError(message) | ||
| warnings.warn(message, UserWarning, stacklevel=2) | ||
| else: | ||
| self.dynamics_model.load_state_dict(state_dict["dynamics_model"], | ||
| strict=strict) | ||
| super().load_state_dict(state_dict, strict=strict) |
There was a problem hiding this comment.
This feels like a patch to an upstream bug; the dynamics model should be part of the state dict (?). Did we forget an ``nn.Module'' somewhere?
There was a problem hiding this comment.
Yeah, I think that's a fair point. From what I understand the issue is that Solver has it's own state_dict method that saves self.model, while the DCL puts the dynamics module in self.reference_model = Sequential(model, dynamics), so the issue is that the custom saving never saves reference_model (DCL still needs model as a separate attribute to compute the embeddings and the pos/neg pairs etc...). I guess one clean option would be to add a custom state_dict and load_state_dict methods to AuxiliaryVariableSolver (which DCL inherits) that save and load reference_model. Otherwise one modifies the current method of the base Solver to handle the optional reference_model if present... what do you think @stes ?
A DCL estimator saved with the sklearn backend restores its encoder on load but leaves its dynamics model freshly initialized. Solver checkpoints omitted the dynamics parameters, so embeddings could match after a round trip while predicted latent transitions changed.
This patch saves and restores the dynamics state for both minibatch and full-batch DCL solvers through a shared mixin. It adds round-trip and resumed-training regression tests for linear and orthogonal-linear dynamics, plus documentation of historical checkpoint behavior.
Reproducer
The failure was independently reproduced on upstream commit
b7e4b9e.Checkpoint compatibility
Historical DCL checkpoints lack the saved dynamics weights. Strict loading raises an explanatory error for those checkpoints. Explicit low-level
solver.load_state_dict(state, strict=False)loads the available components, warns, and retains the initialized dynamics. The change is confined to the DCL solvers.Validation
I have signed the Contributor License Agreement.