Skip to content

Conversation

seanexp
Copy link

@seanexp seanexp commented Mar 17, 2025

What is this PR about

Current implementation of SFTDataset only supports single turn prompt. This PR adds support for multi-turn prompt which is in messages format.

@CLAassistant
Copy link

CLAassistant commented Mar 17, 2025

CLA assistant check
All committers have signed the CLA.

print(f'self.prompts={self.prompts}')
raise
self.prompts = self.prompts.tolist()
self.prompts = self.prompts.squeeze().tolist()
Copy link
Author

Choose a reason for hiding this comment

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

When using pandas==2.2.2, this line raises an error complaining DataFrame does not have .tolist() method. So I added .squeeze() method.

loss_mask[:min(prompt_length, loss_mask.size(0)) - 1] = 0
# mask out the last token in response
loss_mask[min(prompt_length + response_length, loss_mask.size(0)) - 1] = 0
# loss_mask[min(prompt_length + response_length, loss_mask.size(0)) - 1] = 0
Copy link
Author

Choose a reason for hiding this comment

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

I have no idea why eos_token masking is needed so I erased the line. I'll revert this if the masking is needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vermouth1992 could u help take a look?

@seanexp
Copy link
Author

seanexp commented Mar 17, 2025

I am aware of this PR, but I add this PR because the PR is WIP.

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