-
Notifications
You must be signed in to change notification settings - Fork 155
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
Guarantee sorted results from IntentDeterminationEngine.determine_intent #137
Conversation
Enumerate all possible parse results if context or regex entities are in play.
cherry pick of mycroft-core/pull/2915 waiting for MycroftAI/adapt/pull/137 Added to docstring to explain why the method took a list of utterances instead of a single utterance. Fixed a bug where the highest confidence from the Adapt parser is different than the highest confidence from the Adapt intent matcher.
cherry pick of mycroft-core/pull/2915 waiting for MycroftAI/adapt/pull/137 Added to docstring to explain why the method took a list of utterances instead of a single utterance. Fixed a bug where the highest confidence from the Adapt parser is different than the highest confidence from the Adapt intent matcher.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really nice. Should we bump the version all the way to 0.5.0 since this changes an outwardly observable behavior?
My only comment is that this makes Adapt no longer "greedy" when doing the sorting. Is this acceptable? If so, should comments be added to this effect? |
Also updating comment to call out scenarios in which the algorithm is greedy vs when it's more thorough.
@ChristopherRogers1991 updated the comments on IntentDeterminationEngine to incorporate the fact that we're only sometimes greedy. Also bumped to 0.5.0. |
I think you probably meant to tag @chrisveilleux. |
Yup, my bad! Thanks other Chris! |
cherry pick of mycroft-core/pull/2915 waiting for MycroftAI/adapt/pull/137 Added to docstring to explain why the method took a list of utterances instead of a single utterance. Fixed a bug where the highest confidence from the Adapt parser is different than the highest confidence from the Adapt intent matcher.
cherry pick of mycroft-core/pull/2915 waiting for MycroftAI/adapt/pull/137 Added to docstring to explain why the method took a list of utterances instead of a single utterance. Fixed a bug where the highest confidence from the Adapt parser is different than the highest confidence from the Adapt intent matcher.
Enumerate all possible parse results if context or regex entities are in play.
Description of what the PR does, such as fixes # {issue number}
Fixes IntentDeterminationEngine.determine_intent does not return sorted results #136
Description of how to validate or test this PR
Explicit unit test added:
IntentEngineTests.testResultsAreSortedByConfidence
./run_tests.sh
Whether you have signed a CLA (Contributor Licensing Agreement)
Signed