-
Notifications
You must be signed in to change notification settings - Fork 530
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
Add optional logging of text output to EvalOutputLogging #1283
Conversation
@maxisawesome can you please take a look? CC: @dakinggg |
Looks great other than the one comment I left! Thanks for fixing this for non-generative evals. |
@sjawhar if you can fix lint + unit tests (the CPU ones) that would be awesome! then we can merge |
Anything else I can do to help get this merged? |
@sjawhar sorry about that, will take a look this week! |
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, please update the PR title and description to reflect the changes in this PR. Thank you!
Done |
@sjawhar looks like the tests failed with the recent change |
Adds the ability to log text output using
EvalOutputLogging
in non-metrics/ICL use cases. This appears as a newoutputs
key in the logged dictionary, which is set tostats.outpus
and de-tokenized when the model is aHuggingFaceModel
and the dataset has a tokenizer.