rhasspy / rhasspy-rasa-nlu-hermes

MQTT service for natural language understanding in Rhasspy using Rasa NLU with the Hermes protocol
MIT License
3 stars 3 forks source link

Intent handling happening in intent recognition? #3

Open daniele-athome opened 4 years ago

daniele-athome commented 4 years ago

This probably applies to all NLU modules, but let's start from this one.

I noticed that when I "simulate" an intent (e.g. I click "Recognize" withouth checking Handle") the Rasa NLU module publishes hermes/intent/intentName to MQTT. I mean that could already be considered "intent handling" in a way (any MQTT subscriber listening can't know about the "Handle" checkbox in Rhasspy UI).

I believe this behavior should be splitted into a dedicated intent handling module ("Hermes MQTT", it would be a new choice from the "Intent handling" section in settings) that subscribes to hermes/nlu/intentParsed and publishes hermes/intent/intentName.

Does this feel right to you? Am I making any sense?

EDIT: On a second thought, I can see the HomeAssistant module relies on hermes/intent/intentName. I'm not sure how to approach this :thinking:

daniele-athome commented 4 years ago

On a third thought :-) from Snips Dialogue API reference:

This is the main message the handler code should subscribe to. It is sent by the Dialogue Manager when an intent has been detected.

If I understand that sentence correctly, it would mean that:

maxbachmann commented 4 years ago

Yes following the docs I think the NLU should only publish to hermes/nlu/intentParsed: https://docs.snips.ai/reference/hermes#obtaining-the-result-of-an-nlu-parsing-low-level-api

daniele-athome commented 4 years ago

I created some pull requests towards that. I've been running on that in my production environment and it works well. It's missing minimum NLU confidence because it's not ready yet.

EDIT: if those PR are accepted I'll continue adapting the other NLU modules.

maxbachmann commented 4 years ago

@daniele-athome where did you find the specification for https://github.com/rhasspy/rhasspy-hermes/pull/21/files?

daniele-athome commented 4 years ago

@daniele-athome where did you find the specification for https://github.com/rhasspy/rhasspy-hermes/pull/21/files?

I adapted it from the one in NluIntent to make it compatible with the web services.

maxbachmann commented 4 years ago

Ah ok I thought it was a standard hermes one, but could not find it ;)

synesthesiam commented 4 years ago

I don't have a problem with this change in theory, but I want to make sure we don't break expectations.

As I understand it, the main difference between NluIntentParsed and NluIntent is that slot values have been resolved in the latter. In Snips this was done in the dialogue manager, so it made sense that it fired NluIntent. In Rhasspy (currently) the NLU services are wholly responsible for everything NLU related, including slot resolution. Does it make sense to keep this distinction going forward?

More importantly, there's the option in Rhasspy not to have a dialogue manager at all. This means any Rhasspy "apps" will have to be careful about subscribing to NluIntent vs NluIntentParsed because different users may or may not have a dialogue manager.

Thoughts?

koenvervloesem commented 4 years ago

In Snips it was the custom that apps subscribed to the NluIntent message; NluIntentParsedwas a more low-level message that wasn't meant for app developers, as @maxbachmann pointed out.

I agree with @daniele-athome and @maxbachmann that it makes sense to handle the former in the dialogue manager and the latter in the NLU component.

However, @synesthesiam has a good point: slot resolution seems to me like something that needs to be done in the NLU component, it's not related to the dialogue. If we do move slot resolution to the dialogue manager, this means we have to develop a dialogue manager component for each NLU component (Rasa, Snips, ...)

I'm not sure how to solve this...

daniele-athome commented 4 years ago

As I understand it, the main difference between NluIntentParsed and NluIntent is that slot values have been resolved in the latter. In Snips this was done in the dialogue manager, so it made sense that it fired NluIntent. In Rhasspy (currently) the NLU services are wholly responsible for everything NLU related, including slot resolution. Does it make sense to keep this distinction going forward?

I'm not sure I understand... by reading here, I can see that all is done is just putting more or less the same information in both NluIntent and NluIntentParsed. As a matter of fact, what I did in my patches is just listening to NluIntentParsed and copying information over to an NluIntent. Slots filling is still happening in the NLU module, isn't it? NluIntentParsed is enough to generate NluIntent, right? If that's the case, ultimately it's just a matter of responsibility (or maybe I'm missing something).

More importantly, there's the option in Rhasspy not to have a dialogue manager at all. This means any Rhasspy "apps" will have to be careful about subscribing to NluIntent vs NluIntentParsed because different users may or may not have a dialogue manager.

As @koenvervloesem said, NluIntentParsed is low-level API and apps shouldn't subscribe to it unless they know what they are doing. Ultimately it depends on how strict we want our compatibility with Hermes to be.

synesthesiam commented 4 years ago

@daniele-athome, you're correct. In Rhasspy, the NluIntent and NluIntentParsed messages are basically the same because Rhasspy doesn't have an extra "slot resolution" step. The only reason NluIntentParsed exists is for compatibility (HermesLedControl listens to it, for example).

I do see a benefit to having the dialogue manager fire NluIntent since it can then filter out intents below a certain threshold, freeing the NLU services from doing it. This assumes, though, that all NLU services will share the same notion of "confidence" (the different ASR systems certainly don't).

I'm leaning more towards @daniele-athome's proposal of moving NluIntent to the dialogue manager. Any other thoughts?

maxbachmann commented 4 years ago

@synesthesiam I think this would make absolutely sence.

manuelki commented 3 years ago

hi @maxbachmann @koenvervloesem @synesthesiam Thanks for your work on this project. What's the status of this issue, will you push the changes? I also think it would make sense to fire NluIntent from dialogue manager, and thus, stick to the Hermes protocol and the Rhasspy documentation. BTW, do you know of some concise documentation of the Hermes protocol with a proper sequence diagram, which would really help clarify how messages are exchanged between the different components?

maxbachmann commented 3 years ago

BTW, do you know of some concise documentation of the Hermes protocol with a proper sequence diagram, which would really help clarify how messages are exchanged between the different components?

I personally like the original documentation of snips: https://docs.snips.ai/reference/hermes https://snips.gitbook.io/tutorials/t/technical-guides/listening-to-intents-over-mqtt-using-python includes a diagram. It shows in which order messages are exchanged between the different components.