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

Add a way to upload a sentences.ini and retrain Rhasspy. #12

Open
koenvervloesem opened this issue May 25, 2020 · 5 comments
Open

Add a way to upload a sentences.ini and retrain Rhasspy. #12

koenvervloesem opened this issue May 25, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@koenvervloesem
Copy link
Member

koenvervloesem commented May 25, 2020

One of the things on my TODO list for rhasspy-hermes-app is:

Let the app load its intents/slots/… from a file and re-train Rhasspy on installation/startup of the app.

This would make it easier for users of a Rhasspy app to install their app, as they don't have to fiddle themselves anymore with sentences.ini files.

However, I don't believe this is possible yet with the Hermes protocol. I can create a gzipped and pickled intent graph from a sentences.ini with rhasspy-nlu and there are the rhasspy/asr/<siteId>/train and rhasspy/nlu/<siteId>/train topics but they refer to local file names of where Rhasspy is running.

The HTTP API does support uploading sentences with POST to the /api/sentences endpoint. Can we do something like this in MQTT too?

I'm also thinking about security now: we don't want apps to be able to overwrite each other's intent files. So I believe the MQTT topic for this functionality should look like rhasspy/<foobar>/<app_name_or_id>. Then we can restrict the permissions for the app to that specific MQTT topic with their name or ID in to upload/train their sentences.ini in an ACL file for Mosquitto.

And thinking even further: we don't want a random app be able to overwrite a GetTime intent from another app, even if it is in another file. What does Rhasspy currently do when you put two definitions of the same intent in two different sentences.ini files?

To prevent this problem, maybe Rhasspy should preprocess uploaded intents so it adds a sort of namespace to it? The app name or ID used in the MQTT topic could be used as a prefix, so for instance if an app uploads this sentences.ini file with the MQTT API to rhasspy/<foobar>/koen_weather_app:

[GetWeather]
how is the weather
is it raining

Then Rhasspy would create the intent koen_weather_app:GetWeather or something like that:

[koen_weather_app:GetWeather]
how is the weather
is it raining

Does this make sense? There's also a discussion about this on the forum.

@synesthesiam
Copy link
Contributor

With a minor modification to rhasspy-hermes, I was able to get some interesting behavior that could be useful here. A "namespace" can already be added to intents like this:

[koen_weather_app/GetWeather]
how is the weather
is it raining

This trains just fine and will even get recognized/published out on hermes/intents/koen_weather_app/GetWeather -- the web interface just won't pick it up without a small change.

What do you think about just using that for namespacing? We could automatically add <app>/ to the front of intent names, and then an ACL could control hermes/intent/<app>/# for each app.


If you add multiple intents with the same name, they should get combined into one intent (due to how Python's ini parser behaves).


I'm all for suggestions on MQTT training and updating sentences/slots. We want to make sure different services can send in sentences and re-train without stepping on each other's toes, as well as make sure permissions are respected.

@koenvervloesem
Copy link
Member Author

I like the / to delineate the intent namespace. It translates nicely to the MQTT topic hierarchy and it makes an ACL for an app a one-liner. Why didn't I think of that? :-)

@daniele-athome
Copy link

I like it too. Only thing: beware of the existing code (only thing I remember having seen).

@maxbachmann
Copy link
Member

Snips already had namespaces for skills aswell using the format <skill_creator_name>:<intent_name>. However this was always a pain, since the skill author would be different when e.g. someone else used the skill, which would break the application, so I think it is a lot better to use the skill name as you proposed. Beside this using native mqtt namespaces seems like a lot better solution than what snips did 👍

@maxbachmann
Copy link
Member

When we do this we should make sure we only regenerate the sentences of the changed sentences.ini, since generating the sentences can take a long time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants