-
Notifications
You must be signed in to change notification settings - Fork 30.6k
Trainer: Pass num_items_in_batch
to compute_loss
in prediction_step
#41183
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.
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
base: main
Are you sure you want to change the base?
Trainer: Pass num_items_in_batch
to compute_loss
in prediction_step
#41183
Conversation
I think the test case could be better open to hearing suggestions to add tests (for multi-gpu?) or modify the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! Thanks for this nice PR !
….com/pramodith/transformers into pramodith/predict_num_items_in_batch
Hmmm these tests are still failing: tests/trainer/test_trainer.py::TrainerIntegrationTest::test_evaluate_with_jit, can you quickly check why ? I guess the simplest solution would be to check if we have |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Will take a look in a few hours once I'm off work. |
The issue was with |
What does this PR do?
Ensures that
num_items_in_batch
is passed to thecompute_loss
function in theprediction_step
to ensure that loss is calculated the same way both at train and eval time.Fixes #41108
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@SunMarc