Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: allow for padding free + pretraining #426

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: allow for padding free + pretraining #426

wants to merge 1 commit into from
+30 −8

Conversation

HarikrishnanBalagopal
Copy link
Contributor

@HarikrishnanBalagopal HarikrishnanBalagopal commented Dec 20, 2024 •
edited
Loading

Description of the change

Fixes #427

Related issue number

#427

How to verify the PR

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

@github-actions GitHub Actions
Copy link

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

@github-actions github-actions bot added the fix label Dec 20, 2024
fabianlim
fabianlim reviewed Dec 20, 2024
View reviewed changes
tuning/sft_trainer.py Outdated Show resolved Hide resolved
fabianlim
fabianlim requested changes Dec 21, 2024
View reviewed changes
Copy link
Collaborator

@fabianlim fabianlim left a comment

Choose a reason for hiding this comment

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

left suggestions for reducing carrying "padding free" across multiple function calls, leading to better future maintainablity. Also add suggestions to locate padding free check to the config.

tuning/sft_trainer.py Outdated Show resolved Hide resolved
@dushyantbehl
Copy link
Contributor

Also @HarikrishnanBalagopal can you add some unit tests to this scenario in lines of our spirit to have unit testing for new args or features.

dushyantbehl
dushyantbehl reviewed Dec 21, 2024
View reviewed changes
tuning/sft_trainer.py Outdated Show resolved Hide resolved
@HarikrishnanBalagopal
fix: allow for padding free + pretraining
63caead 
Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anhuong anhuong Awaiting requested review from anhuong anhuong is a code owner

@Ssukriti Ssukriti Awaiting requested review from Ssukriti Ssukriti is a code owner

@aluu317 aluu317 Awaiting requested review from aluu317 aluu317 is a code owner

@kmehant kmehant Awaiting requested review from kmehant kmehant is a code owner

@fabianlim fabianlim Awaiting requested review from fabianlim fabianlim is a code owner

@dushyantbehl dushyantbehl Awaiting requested review from dushyantbehl

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

padding_free with extended pretraining causes a crash
3 participants
@HarikrishnanBalagopal @dushyantbehl @fabianlim