Skip to content

intent api #23

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.

Sign up for GitHub

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

Merged
NeonDaniel merged 10 commits into from
Feb 14, 2020
+305 −9
Merged

intent api #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c0e8373
intent api
JarbasAl Feb 3, 2020
28effc4
intent api - manifest
JarbasAl Feb 3, 2020
f70d842
cleanup
JarbasAl Feb 3, 2020
4c34f9a
add regex
JarbasAl Feb 3, 2020
908da21
padatious fix
JarbasAl Feb 3, 2020
101522a
move to intent_service_interface
JarbasAl Feb 3, 2020
b0effa2
get_skill from utterance
JarbasAl Feb 3, 2020
73e3560
fix active skills message
JarbasAl Feb 3, 2020
8fe7fe5
norm utterances
JarbasAl Feb 3, 2020
9d9b495
big cleanup
JarbasAl Feb 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 61 additions & 5 deletions mycroft/skills/intent_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def get_context(self, max_frames=None, missing_entities=None):
relevant_frames[i].entities]
for entity in frame_entities:
entity['confidence'] = entity.get('confidence', 1.0) \
/ (2.0 + depth)
/ (2.0 + depth)
context += frame_entities

# Update depth
Expand Down Expand Up @@ -187,6 +187,7 @@ def __init__(self, bus):

def add_active_skill_handler(message):
self.add_active_skill(message.data['skill_id'])

self.bus.on('active_skill_request', add_active_skill_handler)
self.active_skills = [] # [skill_id , timestamp]
self.converse_timeout = 5 # minutes to prune active_skills
Expand All @@ -197,6 +198,18 @@ def add_active_skill_handler(message):
self.parser_service = TextParsersService(self.bus)
create_daemon(self.parser_service.run)

# Intents API
self.registered_intents = []
self.registered_vocab = []
self.bus.on('intent.service.adapt.get', self.handle_get_adapt)
self.bus.on('intent.service.intent.get', self.handle_get_intent)
self.bus.on('intent.service.skills.get', self.handle_get_skills)
self.bus.on('intent.service.active_skills.get',
self.handle_get_active_skills)
self.bus.on('intent.service.adapt.manifest.get', self.handle_manifest)
self.bus.on('intent.service.adapt.vocab.manifest.get',
self.handle_vocab_manifest)

def update_skill_name_dict(self, message):
"""
Messagebus handler, updates dictionary of if to skill name
Expand Down Expand Up @@ -358,12 +371,13 @@ def handle_utterance(self, message):
# No conversation, use intent system to handle utterance
intent = self._adapt_intent_match(utterances,
norm_utterances,
self.language_config["internal"])
self.language_config[
"internal"])
for utt in combined:
_intent = PadatiousService.instance.calc_intent(utt)
if _intent:
best = padatious_intent.conf if padatious_intent\
else 0.0
best = padatious_intent.conf if padatious_intent \
else 0.0
if best < _intent.conf:
padatious_intent = _intent
LOG.debug("Padatious intent: {}".format(padatious_intent))
Expand All @@ -377,7 +391,7 @@ def handle_utterance(self, message):
{'intent_type': 'converse'})
return
elif (intent and intent.get('confidence', 0.0) > 0.0 and
not (padatious_intent and padatious_intent.conf >= 0.95)):
not (padatious_intent and padatious_intent.conf >= 0.95)):
# Send the message to the Adapt intent's handler unless
# Padatious is REALLY sure it was directed at it instead.
self.update_context(intent)
Expand Down Expand Up @@ -485,6 +499,7 @@ def handle_register_vocab(self, message):
else:
self.engine.register_entity(
start_concept, end_concept, alias_of=alias_of)
self.registered_vocab.append(message.data)

def handle_register_intent(self, message):
intent = open_intent_envelope(message)
Expand Down Expand Up @@ -537,3 +552,44 @@ def handle_remove_context(self, message):
def handle_clear_context(self, message):
""" Clears all keywords from context """
self.context_manager.clear_context()

def handle_get_adapt(self, message):
utterance = message.data["utterance"]
lang = message.data.get("lang", self.language_config["internal"])
norm = normalize(utterance, lang, remove_articles=False)
intent = self._adapt_intent_match([utterance], [norm], lang)
self.bus.emit(message.reply("intent.service.adapt.reply",
{"intent": intent}))

def handle_get_intent(self, message):
utterance = message.data["utterance"]
lang = message.data.get("lang", self.language_config["internal"])
norm = normalize(utterance, lang, remove_articles=False)
intent = self._adapt_intent_match([utterance], [norm], lang)
# Adapt intent's handler is used unless
# Padatious is REALLY sure it was directed at it instead.
padatious_intent = PadatiousService.instance.calc_intent(utterance)
if not padatious_intent and norm != utterance:
padatious_intent = PadatiousService.instance.calc_intent(norm)
if intent is None or (
padatious_intent and padatious_intent.conf >= 0.95):
Copy link
Member

@NeonDaniel NeonDaniel Feb 7, 2020

Choose a reason for hiding this comment

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

Confidence threshold should be read from config so we can make adjustments and keep it in sync with handle_utterance.. Maybe at least put it in a self parameter in this class for now?

Copy link
Collaborator Author

@NeonJarbas NeonJarbas Feb 8, 2020

Choose a reason for hiding this comment

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

this doesnt make sense here, we want it work exactly like in handle_utterance, the whole point is that the intent api behaves exactly like the intent service

notice that this check is only when we have both an adapt intent AND a padatious intent, we only want padatious to take precedence over adapt if it is absolutely sure

if adapt intent is None it always returns the padatious intent, in that case maybe it makes sense to make confidence configurable, which should be done also in the padatious service, that would basically mean a fallback skill is triggered.

Note that padatious service checks for >= 0.8 intents first, then allows fallback skills to try, and then tries again with conf >= 0.5 with lower fallback priority

Either way i think this is out of scope, if we change how padatious works i'd rather do that in a different PR, I'm personally not a big fan of the way mycroft is handling this and want to change it later on. In chatterbox i rewrote this whole piece, we have been talking of open sourcing our "intentBox" repo, in which case it would make sense to also use it here in Neon.

In chatterbox padatious is no longer a fallback skill, IntentBox package takes both kinds of intent into account in a single step

Copy link
Member

@NeonDaniel NeonDaniel Feb 10, 2020

Choose a reason for hiding this comment

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

Right, but if we want to set the padatious_intent.conf threshold to something like 0.8, we would want that to change in both handle_utterance and handle_get_intent, right? I only bring it up because I noticed that we have this confidence level set to 0.8 in neon-core (not sure if/why we made that change) and it could be problematic if handle_get_intent returns something different than handle_utterance..

Copy link
Collaborator Author

@NeonJarbas NeonJarbas Feb 11, 2020

Choose a reason for hiding this comment

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

i think you are confusing things, padatious is using 0.8 for thresh

this 0.95 is only to take priority over adapt

also keep in mind the intent api does not take into account the fallback skills, so if padatious conf < 0.8 a fallback skill might trigger, the intent api will still return the padatious match, consumers need to check the conf manually

we do not need to change anything in here, when the IntentQueryApi checks for a padatious intent it will use the padatious service directly

Copy link
Collaborator Author

@NeonJarbas NeonJarbas Feb 11, 2020

Choose a reason for hiding this comment

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

i don't think this 0.95 thresh should be configurable at all, but if thats something you want i can make it read from config

Copy link
Member

@NeonDaniel NeonDaniel Feb 11, 2020

Choose a reason for hiding this comment

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

Our core has 0.8 hard-coded in intent_service where this PR has 0.95, which why I think this might need to be changed (I can't remember exactly why we made that change, but I think it was related to our i-like-brands skill and some of the brands entities). I just want to make sure that nobody accidentally breaks something in skills by changing 0.95 in one place but misses the other (either by referencing a single var or a comment stating where else the value needs to be constant).

I will do some testing in our core with that value reset to 0.95 because there's a good chance that our change was compensating for some other problem that has since been resolved. I agree with the concept that Adapt will generally yield fewer false positive matches and should take priority unless there's a high confidence Adapt match. I think you're right about leaving it out of the config; opening this up to variable values could make intent matching/troubleshooting across different installations a nightmare..

Copy link
Collaborator Author

@NeonJarbas NeonJarbas Feb 14, 2020

Choose a reason for hiding this comment

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

well, mycroft changed a bit since then and now padatious is given 3 chances to match

  • super confidence, > 0.95 takes priority over adapt
  • high confidence, > 0.8 takes priority over fallback skills
  • low confidence, > 0.5 is a medium low priority fallback skill

I think it makes more sense to look into that value once i start porting all the skills, then if something is not matching properly we should diagnose if it's bad/old intent design or if we need to tweak these 3 confidences

Adapt is always right, in the sense that it is rule-based, so unless a rule is badly written we always want adapt to take precedence, the very high confidence in padatious also means an exact match, in that case since it is matching a full utterance exactly it makes sense to have priority over adapt

intent = padatious_intent.__dict__
self.bus.emit(message.reply("intent.service.intent.reply",
{"intent": intent}))

def handle_get_skills(self, message):
self.bus.emit(message.reply("intent.service.skills.reply",
{"skills": self.skill_names}))

def handle_get_active_skills(self, message):
self.bus.emit(message.reply("intent.service.active_skills.reply",
{"skills": [s[0] for s in
self.active_skills]}))

def handle_manifest(self, message):
self.bus.emit(message.reply("intent.service.adapt.manifest",
{"intents": self.registered_intents}))

def handle_vocab_manifest(self, message):
self.bus.emit(message.reply("intent.service.adapt.vocab.manifest",
{"vocab": self.registered_vocab}))
217 changes: 215 additions & 2 deletions mycroft/skills/intent_service_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
"""The intent service interface offers a unified wrapper class for the
Intent Service. Including both adapt and padatious.
"""
from os.path import exists

from os.path import exists, isfile
from adapt.intent import Intent

from mycroft.messagebus.message import Message
from mycroft.messagebus.client import MessageBusClient
from mycroft.util import create_daemon
from mycroft.util.log import LOG


class IntentServiceInterface:
Expand All @@ -29,6 +31,7 @@ class IntentServiceInterface:
for easier interaction with the service. It wraps both the Adapt and
Precise parts of the intent services.
"""

def __init__(self, bus=None):
self.bus = bus
self.registered_intents = []
Expand Down Expand Up @@ -159,6 +162,216 @@ def get_intent(self, intent_name):
return None


class IntentQueryApi:
"""
Query Intent Service at runtime
"""

def __init__(self, bus=None, timeout=5):
if bus is None:
bus = MessageBusClient()
create_daemon(bus.run_forever)
self.bus = bus
self.timeout = timeout

def get_adapt_intent(self, utterance, lang="en-us"):
""" get best adapt intent for utterance """
msg = Message("intent.service.adapt.get",
{"utterance": utterance, "lang": lang},
context={"destination": "intent_service",
"source": "intent_api"})

resp = self.bus.wait_for_response(msg,
'intent.service.adapt.reply',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["intent"]

def get_padatious_intent(self, utterance, lang="en-us"):
""" get best padatious intent for utterance """
msg = Message("intent.service.padatious.get",
{"utterance": utterance, "lang": lang},
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.padatious.reply',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["intent"]

def get_intent(self, utterance, lang="en-us"):
""" get best intent for utterance """
msg = Message("intent.service.intent.get",
{"utterance": utterance, "lang": lang},
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.intent.reply',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["intent"]

def get_skill(self, utterance, lang="en-us"):
""" get skill that utterance will trigger """
intent = self.get_intent(utterance, lang)
if not intent:
return None
# retrieve skill from munged intent name
if intent.get("name"): # padatious
return intent["name"].split(":")[0]
if intent.get("intent_type"): # adapt
return intent["intent_type"].split(":")[0]
return None # raise some error here maybe? this should never happen

def get_skills_manifest(self):
msg = Message("intent.service.skills.get",
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.skills.reply',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["skills"]

def get_active_skills(self):
msg = Message("intent.service.active_skills.get",
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.active_skills.reply',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["skills"]

def get_adapt_manifest(self):
msg = Message("intent.service.adapt.manifest.get",
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.adapt.manifest',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["intents"]

def get_padatious_manifest(self):
msg = Message("intent.service.padatious.manifest.get",
context={"destination": "intent_service",
"source": "intent_api"})
resp = self.bus.wait_for_response(msg,
'intent.service.padatious.manifest',
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None
return data["intents"]

def get_intent_manifest(self):
padatious = self.get_padatious_manifest()
adapt = self.get_adapt_manifest()
return {"adapt": adapt,
"padatious": padatious}

def get_vocab_manifest(self):
msg = Message("intent.service.adapt.vocab.manifest.get",
context={"destination": "intent_service",
"source": "intent_api"})
reply_msg_type = 'intent.service.adapt.vocab.manifest'
resp = self.bus.wait_for_response(msg,
reply_msg_type,
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None

vocab = {}
for voc in data["vocab"]:
if voc.get("regex"):
continue
if voc["end"] not in vocab:
vocab[voc["end"]] = {"samples": []}
vocab[voc["end"]]["samples"].append(voc["start"])
return [{"name": voc, "samples": vocab[voc]["samples"]}
for voc in vocab]

def get_regex_manifest(self):
msg = Message("intent.service.adapt.vocab.manifest.get",
context={"destination": "intent_service",
"source": "intent_api"})
reply_msg_type = 'intent.service.adapt.vocab.manifest'
resp = self.bus.wait_for_response(msg,
reply_msg_type,
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None

vocab = {}
for voc in data["vocab"]:
if not voc.get("regex"):
continue
name = voc["regex"].split("(?P<")[-1].split(">")[0]
if name not in vocab:
vocab[name] = {"samples": []}
vocab[name]["samples"].append(voc["regex"])
return [{"name": voc, "regexes": vocab[voc]["samples"]}
for voc in vocab]

def get_entities_manifest(self):
msg = Message("intent.service.padatious.entities.manifest.get",
context={"destination": "intent_service",
"source": "intent_api"})
reply_msg_type = 'intent.service.padatious.entities.manifest'
resp = self.bus.wait_for_response(msg,
reply_msg_type,
timeout=self.timeout)
data = resp.data if resp is not None else {}
if not data:
LOG.error("Intent Service timed out!")
return None

entities = []
# read files
for ent in data["entities"]:
if isfile(ent["file_name"]):
with open(ent["file_name"]) as f:
lines = f.read().replace("(", "").replace(")", "").split(
"\n")
samples = []
for l in lines:
samples += [a.strip() for a in l.split("|") if a.strip()]
entities.append({"name": ent["name"], "samples": samples})
return entities

def get_keywords_manifest(self):
padatious = self.get_entities_manifest()
adapt = self.get_vocab_manifest()
regex = self.get_regex_manifest()
return {"adapt": adapt,
"padatious": padatious,
"regex": regex}


def open_intent_envelope(message):
"""Convert dictionary received over messagebus to Intent."""
intent_dict = message.data
Expand Down
Loading