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.

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

Ignore intent/entity prediction if input is empty #10604

Closed
wants to merge 9 commits into from
Closed

Conversation

jupyterjazz
Copy link
Contributor

@jupyterjazz jupyterjazz commented Dec 29, 2021

Proposed changes:

Status (please check what you already did):

  • added some tests for the functionality
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@jupyterjazz jupyterjazz requested review from a team and dakshvar22 and removed request for a team December 29, 2021 12:16
@jupyterjazz jupyterjazz changed the title Ignore intent/entity prediction if an empty string is given Ignore intent/entity prediction if input is empty Dec 29, 2021
@dakshvar22
Copy link
Contributor

What would happen inside core ensemble (for action prediction) if the input message is empty and hence the intent is missing?

@dakshvar22
Copy link
Contributor

Also, if there was an original issue for which you opened the PR, could you please link it in the PR description?

@jupyterjazz
Copy link
Contributor Author

What would happen inside core ensemble (for action prediction) if the input message is empty and hence the intent is missing?

In that case, it's considered as nlu_fallback.

I also checked this with a default bot and looking at the trackers_as_states

trackers_as_states = self.prediction_states(

which is used by policies for prediction, empty string gets represented the same way as it would be without this change.
{'user': {'intent': 'nlu_fallback'}, 'prev_action': {'action_name': 'action_listen'}}

@dakshvar22
Copy link
Contributor

Okay, that makes sense. Another question: Looking at the original issue, it seems like the bug only comes up when you add MitieFeaturizer to the config. If the only change that was needed to fix the issue was in DIETClassifier, then why does the issue not come up in the config which does not have MitieFeaturizer in the config (for e.g. - rasa init config).

@jupyterjazz
Copy link
Contributor Author

then why does the issue not come up in the config which does not have MitieFeaturizer in the config

The reason is that for an empty string MitieFeaturizer doesn't create features which are expected during intent/entity prediction.

@dakshvar22
Copy link
Contributor

Can you please also check if the fix that you added to DIET needs to be added to CRFEntityExtractor and ResponseSelector?

@github-actions
Copy link
Contributor

🚀 A preview of the docs have been deployed at the following URL: https://10604--rasahq-docs-rasa-v2.netlify.app/docs/rasa

tests/nlu/classifiers/test_diet_classifier.py Show resolved Hide resolved
@@ -0,0 +1 @@
If NLU gets an empty string, ignore predicting intents and entities. Some components can't handle empty strings (e.g `MitieFeaturizer`) which causes errors during intent prediction.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If NLU gets an empty string, ignore predicting intents and entities. Some components can't handle empty strings (e.g `MitieFeaturizer`) which causes errors during intent prediction.
Fixes a bug which caused machine learning components like `DIETClassifier`, `ResponseSelector`, `CRFEntityExtractor` and `SklearnIntentClassifier` to throw an error when a message with an empty text string was passed to them for processing.

@dakshvar22
Copy link
Contributor

@jupyterjazz Almost there ✨ A couple of things:

  1. I see that the original issue was tagged as a bug, so this should go out as a bugfix in a patch release. Hence, the PR should target 3.0.x branch and should be forked from that branch. Currently its main.
  2. Once you have done the changes, please run model regression tests overnight so that we are certain that inference is not broken.

@jupyterjazz jupyterjazz deleted the empty_10504 branch January 16, 2022 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty (or some spaces) input text causes Runtime error
2 participants