Skip to content

Preserve DCL dynamics parameters in checkpoints - #312

Open
myscience wants to merge 1 commit into
AdaptiveMotorControlLab:mainfrom
myscience:fix-dcl-checkpoint
Open

myscience wants to merge 1 commit into
AdaptiveMotorControlLab:mainfrom
myscience:fix-dcl-checkpoint

Conversation

@myscience

Copy link
Copy Markdown
Collaborator

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

import numpy as np
import torch
from cebra.integrations.sklearn.cebra import CEBRA, DCL

torch.manual_seed(4)
X = np.random.default_rng(4).normal(size=(100, 5)).astype("float32")
model = DCL(
    device="cpu", output_dimension=3, max_iterations=2,
    batch_size=8, batch_size_negatives=11,
).fit(X)
z = torch.randn(7, 3)
before = model.dynamics_model_(z).detach().clone()
model.save("dcl.pt", backend="sklearn")
restored = CEBRA.load("dcl.pt", backend="sklearn")

# The encoder round trip already works.
np.testing.assert_allclose(model.transform(X), restored.transform(X))
# Fails before this patch; passes with it.
torch.testing.assert_close(restored.dynamics_model_(z), before)

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

  • Full required CPU suite: 3,187 passed, 102 skipped, zero failures; 6,965 dataset-dependent cases deselected.
  • Regression coverage includes both DCL batch modes, linear and orthogonal-linear dynamics, encoder/dynamics round trips, resumed training, and strict/non-strict handling of incomplete checkpoints.
  • Formatting/import ordering, spelling, docstring coverage, citation validation, binary checks, package build, and an installed-wheel checkpoint smoke test passed.
  • Local environment: Python 3.10.12, PyTorch 2.5.1, scikit-learn 1.7.2, NumPy 2.2.6, CPU.

I have signed the Contributor License Agreement.

Copilot AI balanced review requested due to automatic review settings October 23, 2026 16:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@myscience myscience added the bug Something isn't working label Oct 23, 2026
@myscience
myscience requested a review from stes October 23, 2026 16:44
Comment on lines +228 to +250
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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants