MycroftAI / adapt

Adapt Intent Parser
Apache License 2.0
710 stars 154 forks source link

IntentDeterminationEngine.determine_intent does not return sorted results #136

Closed clusterfudge closed 3 years ago

clusterfudge commented 3 years ago

Describe the bug When using regex entities or context, IntentDeterminationEngine.determine_intent does not yield sorted results. Adapt expects that maximal scoring is based on coverage, however this assumption is invalidated when the confidence of matching entities is not consistent (as is the case with regex matches, which are weighted down, and context matches, which are weighted up).

To Reproduce Steps to reproduce the behavior: From @forslund 's gist: https://gist.github.com/forslund/e077b749278de3476ef121b81277d689

from adapt.intent import IntentBuilder
from adapt.engine import IntentDeterminationEngine

def check_for_location(intent):
    if 'Location' in intent:
        print('Location: {}'.format(intent['Location']))
    else:
        print('Location: Not found')

e = IntentDeterminationEngine()

print('Registering weather skill entities')
# Weather Keywords
e.register_entity('what is', 'Query', None)
e.register_entity('weather', 'Weather', None)
e.register_regex_entity('(at|in) (?P<Location>.+)')

print('Registering Current weather intent')

# Registering current weather intent
i = IntentBuilder("CurrentWeatherIntent").require(
                  "Weather").optionally("Location").build()
e.register_intent_parser(i)

print('TESTING "what is the weather like in stockholm"')
best_intent = next(e.determine_intent("what is the weather like in stockholm",
                                      100, include_tags=True))
check_for_location(best_intent)

print('\nRegistering additional regexes...')
# Register additonal regexes
e.register_regex_entity('(read (out )?)?((status|state|value|sensor) )?(?P<Entity>.*)')

print('TESTING "what is the weather like in stockholm"')
best_intent = next(e.determine_intent("what is the weather like in stockholm",
                                      100, include_tags=True))
check_for_location(best_intent)

Expected behavior Both the first and second check_for_location calls should find a Location entity.

Log files Not applicable

Environment (please complete the following information): Not applicable

Additional context This has been at the core of a couple of unmet-expectations style bugs in Adapt and mycroft-core. I believe that band-aids built on top of this should continue to work, however will rely on VK tests to validate that assumption. If not, we should make this opt-in (which would be very sad, to opt in to "hey it works right").