Skip to content

Fix static call#7

Merged
nitsanluke merged 2 commits into
mainfrom
bug_fix_get_commited_iter_numbers
Oct 17, 2024
Merged

Fix static call#7
nitsanluke merged 2 commits into
mainfrom
bug_fix_get_commited_iter_numbers

Conversation

@nitsanluke
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Collaborator

@tscholak tscholak left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread tools/push_model.py Outdated
commits = [c.split(" ", 1)[1] for c in commits]
# Keep commits corresponding to a new iter
return [iter_number for c in commits if (iter_number := self.get_iter_number(c)) is not None]
return [iter_number for c in commits if (iter_number := PushConfig._get_iter_number(c)) is not None]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor nitpick: this should probably a class method instead of a static method. Then:

Suggested change
return [iter_number for c in commits if (iter_number := PushConfig._get_iter_number(c)) is not None]
return [iter_number for c in commits if (iter_number := cls._get_iter_number(c)) is not None]

Copy link
Copy Markdown
Contributor Author

@nitsanluke nitsanluke Oct 17, 2024

Choose a reason for hiding this comment

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

@tscholak cls not defined? or are you suggesting to change _get_commited_iter_numbers to class a method and use self?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks made the method into a classmethod.

@nitsanluke nitsanluke merged commit 3ee5497 into main Oct 17, 2024
@nitsanluke nitsanluke deleted the bug_fix_get_commited_iter_numbers branch October 17, 2024 14:40
@tscholak tscholak added this to the 0.2.0 milestone Oct 25, 2024
jlamypoirier added a commit that referenced this pull request May 6, 2026
- Drop unused self._preprocessing_config store in Trainer.setup.
- Replace torch.ones + index_add_ with torch.bincount for tok_sum
  in fused_gspo_loss_forward_backward.
- Drop load-bearing-sounding docs_per_step reference from the
  normalize_by_documents field description (no cross-config check
  exists to enforce it).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants