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

ZeroDivisionError in determine_intent when tags are empty #114

Closed
PLNech opened this issue Sep 22, 2020 · 1 comment · Fixed by #128
Closed

ZeroDivisionError in determine_intent when tags are empty #114

PLNech opened this issue Sep 22, 2020 · 1 comment · Fixed by #128

Comments

@PLNech
Copy link
Contributor

PLNech commented Sep 22, 2020

  • I'm using an IntentDeterminationEngine along the lines of the Multi Intent parser Example, with no required entity type and several optional ones.

Stacktrace

  File "/home/pln/.PyCharm2019.3/config/scratches/scratch_12.py", line 16, in <module>
    print([i for i in engine.determine_intent(utterance, include_tags=False)])
  File "/home/pln/.PyCharm2019.3/config/scratches/scratch_12.py", line 16, in <listcomp>
    print([i for i in engine.determine_intent(utterance, include_tags=False)])
  File "/home/pln/.virtualenvs/nluservice/src/adapt-parser/adapt/engine.py", line 131, in determine_intent
    best_intent, tags = self.__best_intent(result, remaining_context)
  File "/home/pln/.virtualenvs/nluservice/src/adapt-parser/adapt/engine.py", line 81, in __best_intent
    i, tags = intent.validate_with_tags(parse_result.get('tags') + context_as_entities, parse_result.get('confidence'))
  File "/home/pln/.virtualenvs/nluservice/src/adapt-parser/adapt/intent.py", line 190, in validate_with_tags
    total_confidence = intent_confidence / len(tags) * confidence
ZeroDivisionError: float division by zero

Steps to replicate the Issue

  1. Define an IntentDeterminationEngine as such:
    engine: IntentDeterminationEngine = IntentDeterminationEngine()
    engine.register_entity("Kevin", "who") # same problem if several entities
    builder: IntentBuilder = IntentBuilder("Buddies")
    builder.optionally("who") # same problem if several entity types
    engine.register_intent_parser(builder.build())
  1. Use it on an utterance that contains an entity, get expected results
print([i for i in engine.determine_intent("Kevin is a friend")]) # works
  1. Use it on an utterance that contains no entity, get an exception
print([i for i in engine.determine_intent("Julien is a friend")]) # raises

Can also be reproduced with an empty utterance "".

Be as specific as possible about the expected condition, and the deviation from expected condition.

I expect the engine to return an empty determination ([]) when no entity is recognized, instead of raising a generic ZeroDivisionError exception.

@PLNech
Copy link
Contributor Author

PLNech commented Sep 22, 2020

I believe fixing this could just mean refactoring validate_with_tags. For now I'd see this as:

  • Update the line 190 that divides by the length of a potentially empty collection:

    total_confidence = intent_confidence / len(tags) * confidence

    • Could be wrapped in a conditional, e.g. total_confidence = [..] or 0 if not tags
  • Take the opportunity to clarify the role of the confidence parameter, documented as a float:

    adapt/adapt/intent.py

    Lines 137 to 142 in 402d2d8

    def validate_with_tags(self, tags, confidence):
    """Validate whether tags has required entites for this intent to fire
    Args:
    tags(list): Tags and Entities used for validation
    confidence(float): ?

    From my understanding, its value is always ignored, being redefined here:

    required_tag, canonical_form, confidence = find_first_tag(local_tags, require_type)

    Its final value is used to compute the total confidence in the line 190 I linked above. Thus, the new value of confidence defined line 192 is never used:

    target_client, canonical_form, confidence = find_first_tag(local_tags, CLIENT_ENTITY_NAME)

    • Could remove confidence from validate_with_tags's parameters as it is never used
    • Could replace confidence by _ in line 192 as it is never used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants