Skip to content

[Feature] Add CI test for weight version update#1120

Merged
zhuzilin merged 3 commits intoTHUDM:mainfrom
yitianlian:ci4_version
Dec 16, 2025
Merged

[Feature] Add CI test for weight version update#1120
zhuzilin merged 3 commits intoTHUDM:mainfrom
yitianlian:ci4_version

Conversation

@yitianlian
Copy link
Collaborator

No description provided.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 46 to +47
def update_weights(self) -> None:
self.weight_version += 1

Choose a reason for hiding this comment

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

P1 Badge Initialize weight_version for distributed weight updater

UpdateWeight.update_weights now increments self.weight_version (lines 46‑47), but UpdateWeightFromDistributed overrides __init__ without calling the base constructor, so instances created when args.colocate is False never define weight_version. The next call to update_weights() raises AttributeError before any tensors are sent, breaking non-colocated training runs. Make sure UpdateWeightFromDistributed sets self.weight_version (e.g., by invoking super().init) so distributed weight sync can run.

Useful? React with 👍 / 👎.

@zhuzilin zhuzilin merged commit e0e840d into THUDM:main Dec 16, 2025
10 checks passed
Fengzdadi pushed a commit to Fengzdadi/slime that referenced this pull request Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments