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

config.load_langs_on_demand: permits on-the-fly loading/unloading, to facilitate proc-gen use cases #158

Merged
merged 5 commits into from
Nov 29, 2020

Conversation

ChanceNCounter
Copy link
Contributor

This branch (squash pls) adds config.py (eventually #128) with one variable: load_langs_on_demand.

When this variable is True, the localized function caller will - if and only if a language is passed which is not loaded - load the language, call the function, and unload the language.

At the moment, this is pretty fast, though it might need to be revisited if some languages become truly bloated with resource files.

This is only desirable in certain use cases, but, for instance, the absence of this flag was an obstacle for @JarbasAl's modular text2speech package, which doesn't always know the language it wants ahead of time.

I've tested this PR against that module, and it solves the problem. I want to stress that use cases like Mycroft, which prefer to utilize at most one engine per language, will not want to set this flag.

Adds config.py to hold this variable (see also:
MycroftAI/lingua-franca/MycroftAI#128)
@ChanceNCounter ChanceNCounter added enhancement New feature or request multi_lang relates to several languages labels Nov 24, 2020
@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Nov 24, 2020
Copy link
Collaborator

@JarbasAl JarbasAl left a comment

Choose a reason for hiding this comment

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

I approve of this PR, as discussed in chat with @ChanceNCounter sometimes we will not know what language will be needed, and in these cases the lang parameter will be passed along.

Since the user is explicitly requesting a language, he wants it loaded for sure, however as @ChanceNCounter pointed out this would bloat memory usage, eg, normalize(utt, lang="pt") for a single utterance would keep pt language data in memory, even if pt is never used again. Unloading makes perfect sense. It's up for end users to explicitly load a language if they want the performance benefits (speed) from not having to load everytime.

At same time this avoids the need for a pattern of calling load_language, func, unload_language every time we use a function

@krisgesling
Copy link
Contributor

Yeah sounds like a good change, agree that the intent is there if the lang param is passed.

If Jarbas has reviewed the code I'm a +1

My only question is whether we can we add a test to cover this use case?

@ChanceNCounter
Copy link
Contributor Author

Yeah, that's a dumb oversight! On it.

ensure that load_langs_on_demand doesn't unload langs that were already
loaded
@JarbasAl JarbasAl merged commit d74e37b into MycroftAI:master Nov 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) enhancement New feature or request multi_lang relates to several languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants