-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Tests for invalid phonemes fallbacks fail on latest pocketsphinx #2574
Comments
Python 3.8 passes all tests on Travis, gonna test it locally as well to confirm. Do you know which version of pocketsphinx and python-pocketsphinx is being used? |
0.1.15, which is probably the problem here, being 15 point releases further 😉 |
Did a quick test and upgraded pocketsphinx resulting in me getting the exact same issue so definitely the issue. |
Cool, at least I now know where to look. I'll investigate! To be honest I'm amazed this is the only problem that comes with the newer versions, seeing how strict Mycroft normally keeps their dependencies |
Lol yeah, me too! We've had our fair share of breakage though when doing updates (py2 to 3 was rough, and upgrading from the arcane tornado version to the newer one was quite annoying) |
Interesting enough release 0.1.5 through 0.1.15 was done in a space of 3 days, and the tests still break on 0.1.3 which was the first release available on PyPi after 1.0. 0.1.0 is actually from June 2016, so I guess the dep never has been upgraded in mycroft-core since the public release? At least now I know the breakage is somewhere between the 4th of June 2016 and the 12th of September 2016 (between 0.1.0 and 0.1.3). |
Hmpf, the problem isn't in pocketsphinx-python itself, it's in the submodule pocketsphinx it uses. If I use pocketsphinx-python 0.1.0 but with newer submodules, the tests break in the same way. But if I use it with the submodules that come with that release, it works fine. I guess that is the issue of using libraries without releases... In fact, the breakage is between https://github.com/cmusphinx/pocketsphinx/commits/ec89abd9d2e976019338ecae7d6cafeecdb95eb2 and https://github.com/cmusphinx/pocketsphinx/commits/a60982363101704eca342e7e0920754090cd49b1 pocketsphinx-python 0.1.15 with the submodules from 0.1.0 actually makes these tests succeed! |
Gotcha, cmusphinx/pocketsphinx@08e367e is the commit that broke it. I'm not sure how this breaks things, but maybe it's obvious to someone? |
Ah, seems https://github.com/MycroftAI/mycroft-core/blob/dev/mycroft/client/speech/hotword_factory.py#L108 is outdated and should now be cmusphinx/pocketsphinx@08e367e#diff-0376830e1b8c31e50aa1f84206085a41 |
Nice find, so instead of passing a keyphrase we need to write the keyphrase and threshold to a tmp file and pass that in? Or is it somehow similar to the -dict file that's used to pass in phonemes and word mappings (I think it is atleast)? |
Well the test file used by pocketsphinx contains just a few things:
So it seems like it can be a tmp file, but I'm not sure, I don't know pocketsphinx 😂 |
I did a test changing the |
Hmm https://cmusphinx.github.io/wiki/faq/ suggests that -keyphrase still is valid... |
Hmm maybe. I guess Then it probably goes wrong in verifying the keyphrase it's passed, since that logic has changed. |
If you got other things to dig into I can try to spend some time finding some solution to this tomorrow or so. |
I have tons of things to do haha. Main problem here is that this is a bit out of my league, so I would love it if you could take a look at it. |
@forslund did you have any luck with this by any chance? |
I haven't gotten all the way through it. I may have a workaround half-done (if it works) |
@forslund how is your workaround going? |
Well my first attemt didn't work at all. Sort of forgot about it recently, sorry. Gonna give it some extra attention... |
I'm running the tests on Alpine Linux using system packages and Python 3.8.
I currently have all tests succeeding except for
PocketSphinxTest.testInvalid
andLocalRecognizerInitTest.testListenerConfig
. It seems the fallback for invalid phonemes isn't working.The text was updated successfully, but these errors were encountered: