transformers icon indicating copy to clipboard operation
transformers copied to clipboard

Add hallucination filter

Open KMFODA opened this issue 2 years ago • 2 comments

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

KMFODA avatar Aug 18 '22 07:08 KMFODA

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.

github-actions[bot] avatar Sep 17 '22 15:09 github-actions[bot]

Hi @KMFODA are you still planning to work on this? We can reopen the PR :)

gante avatar Sep 27 '22 11:09 gante

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 avatar Sep 28 '22 08:09 KMFODA

@KMFODA absolutely no rush, take your time -- personal health always takes precedence! I hope you're feeling better 🤗

gante avatar Sep 28 '22 13:09 gante

(@KMFODA let us know when you'd like a new review)

gante avatar Oct 10 '22 09:10 gante

Thanks @gante. Just managed to get all the tests to pass so a review now would be much appreciated.

KMFODA avatar Oct 11 '22 11:10 KMFODA

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?

KMFODA avatar Nov 10 '22 11:11 KMFODA

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 avatar Nov 14 '22 19:11 KMFODA

@KMFODA did this processor help with your use case? :)

gante avatar Nov 14 '22 19:11 gante

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.

KMFODA avatar Nov 15 '22 16:11 KMFODA

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.

KMFODA avatar Nov 29 '22 09:11 KMFODA

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?

KMFODA avatar Nov 30 '22 14:11 KMFODA

Managed to fix the failing test by rebasing to main. Hopefully should be good to merge now but if not let me know!

KMFODA avatar Dec 01 '22 11:12 KMFODA

Cool let's just ask for a final review from @sgugger ! 🤗

ArthurZucker avatar Dec 05 '22 09:12 ArthurZucker

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 :)

gante avatar Dec 15 '22 20:12 gante

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.

KMFODA avatar Dec 28 '22 14:12 KMFODA

Hey @gante, moved encoder_repetition_penalty to GenerationConfig and fixed all failing tests. Let me know if more is needed to merge this PR.

KMFODA avatar Jan 05 '23 12:01 KMFODA

Thanks @gante helpful changes. Just implemented and fixed failing tests.

KMFODA avatar Jan 06 '23 07:01 KMFODA

@KMFODA awesome, thanks 🙏

@sgugger this PR should be ready to go in, feel free to merge if you are also happy with it :)

gante avatar Jan 16 '23 11:01 gante

Hopefully should be good to merge now. If not let me know.

KMFODA avatar Jan 19 '23 09:01 KMFODA

Thanks for your contribution!

sgugger avatar Jan 19 '23 16:01 sgugger