Skip to content

Conversation

@jlamypoirier
Copy link
Collaborator

✨ Description


# Policy loss
# TODO: advantages or rewards?
log_ratio_old = torch.exp(target_logprobs - old_logprobs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this would read better as ratio_new_old = torch.exp(target_logprobs - old_logprobs)

target_logprobs = torch.gather(logprobs, dim=2, index=labels.unsqueeze(2)).squeeze(2)

# Policy loss
# TODO: advantages or rewards?
Copy link
Collaborator

Choose a reason for hiding this comment

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

answer: advantages

)

# TODO: tokens_weights = 1/batch_size ?
# TODO: Reduce loss?
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to sum over tokens and apply mask

torch.clamp(log_ratio_old, 1 - self.epsilon_low, 1 + self.epsilon_high) * advantage,
)

# TODO: tokens_weights = 1/batch_size ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think so, we do that for the simple case


# TODO: tokens_weights = 1/batch_size ?
# TODO: Reduce loss?
loss = loss / batch_size # 1 x (BxL) x 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

loss = -loss - we want to maximise the objective

Base automatically changed from jlp_entropy_loss to main January 22, 2026 21:57
@jlamypoirier jlamypoirier changed the base branch from main to jlp_pipeline_rl January 22, 2026 23:38
@jlamypoirier jlamypoirier marked this pull request as ready for review January 22, 2026 23:40
@jlamypoirier jlamypoirier merged commit 39356fb into jlp_pipeline_rl Jan 22, 2026
1 of 2 checks passed
@jlamypoirier jlamypoirier deleted the jlp_grpo branch January 22, 2026 23:40
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.

4 participants