transformers
transformers copied to clipboard
Add hallucination filter
What does this PR do?
As per the discussions in https://github.com/huggingface/transformers/issues/18354, this PR aims to add a HallucinationPenaltyLogitsProcessor
that takes in a hallucination_penalty
that is applied to any tokens that are not in the original input. This acts as a hallucination filter and the higher it is set the more likely the text is going to contain only the input tokens. For summarisation this means the higher the hallucination penalty the more extractive the summary is going to be and the less likely it is to have a hallucination.
Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the contributor guideline, Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the forum? Please add a link to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the documentation guidelines, and here are tips on formatting docstrings.
- [ ] Did you write any new necessary tests?
Who can review?
@gante, @patrickvonplaten
Library:
- text generation: @patrickvonplaten
The documentation is not available anymore as the PR was closed or merged.
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.
Hi @KMFODA are you still planning to work on this? We can reopen the PR :)
Hey @gante yes I still plan to work on this. My apologies I had fallen ill this past month and couldn't spend time on this. If you re-open the PR I will prioritise working on this over the next few weeks.
@KMFODA absolutely no rush, take your time -- personal health always takes precedence! I hope you're feeling better 🤗
(@KMFODA let us know when you'd like a new review)
Thanks @gante. Just managed to get all the tests to pass so a review now would be much appreciated.
Hi @gante, I added a test_encoder_repetition_penalty_dist_process
to cover the 1st type of test. The 2nd test you've linked seems to be more focused on beam searches and stopping criteria. What type of test did you have in mind here for the encoder_repetition_penalty? Would ensuring it's initialised by adding it to this test cover this?
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.
Done, thanks for all the helpful comments to get this merged and apologies it took so long.
@KMFODA did this processor help with your use case? :)
Thanks for asking @gante! It worked yes, although I had to use a very small penalty to eventually remove the hallucination. In a call with Karim and Joao (changing the names to protect the real data) the model I'm using generates the following action:
Tom will give Joao his email address.
Where Tom is a hallucination and an individual not in this call. After applying a penalty of 0.001 I get the following output:
Karim will send Joao an email today.
Hey @gante, let me know if anything else is needed to get this merged. Using this in the inference pipeline / API would be of really helpful.
Thanks @ArthurZucker. Added the doc strings in the 3 different files you mentioned. I've only got one test failing which I can't recreate locally:
tests/pipelines/test_pipelines_zero_shot.py::ZeroShotClassificationPipelineTests::test_small_model_tf
It seems like the outputs changed in the zero-shot-classification
pipeline although I'm not sure why. Are you able to point me towards what might be causing this to fail?
Managed to fix the failing test by rebasing to main. Hopefully should be good to merge now but if not let me know!
Cool let's just ask for a final review from @sgugger ! 🤗
Hey @KMFODA -- the big change we just merged clashed with your PR, as @sgugger mentioned above.
In a nutshell, new generation parameters should go in GenerationConfig
(here), and generate always uses a generation config (related docs). We need to change this PR to make it consistent with the new changes :D
I understand this PR has been a long process, so it's okay if you don't want to make further changes. Just let me know if you are no longer interested in working on it :)
Hey @gante thanks for getting back to me. Not a problem, I'd still like to work on this as it'll help me learn about the new generation engine. I'll get working on this after the holidays.
Hey @gante, moved encoder_repetition_penalty to GenerationConfig
and fixed all failing tests. Let me know if more is needed to merge this PR.
Thanks @gante helpful changes. Just implemented and fixed failing tests.
@KMFODA awesome, thanks 🙏
@sgugger this PR should be ready to go in, feel free to merge if you are also happy with it :)
Hopefully should be good to merge now. If not let me know.
Thanks for your contribution!