Closed sfgeorge closed 7 years ago
@gfaza Can you review this for me?
Does Lumenvox not have their own pare-compiled built-in grammar for this? These grammars were intended for use with Punchblock's recogniser.
tl;dr Here's a an attempt at a rationale for this PR. Let me know whether or not this sounds reasonable and if so I'm happy to make the little tweaks to make hound-ci happy. Thanks man!
Does Lumenvox not have their own pare-compiled built-in grammar for this? These grammars were intended for use with Punchblock's recogniser.
Good question there. LumenVox does have a builtin for this, but their builtins are compiled and cached on demand, and are optimized no more than any custom grammar that you pass in (this is good). So one's custom grammars are optimized and cached with the same priority as their own builtins, which is nice.
One benefit for using a ruby_speech number dtmf grammar is that for customers who sometimes use ASR and sometimes use DTMF-only (such as myself), there is a consistent number dtmf grammar being used.
Additionally, having more control over your grammars, such as when you use ruby_speech, is especially helpful when there are issues with the builtins provided by vendors. Due to performance problems I encountered, I can share that LumenVox support has recommended to me to stop using their builtin number grammar. They encourage customers to use finite sequences rather than infinite whenever possible. Their builtin number grammar itself is currently inifite and can cause trouble. Now, this PR continues to support an infinite series for a number question, but there are 2 reasons why this is still helpful:
RubySpeech::GRXML::Builtins.number
and its use in Punchblock have superb support for passing in future options such as maxlength, much like to how RubySpeech::GRXML::Builtins.decimal
already supports maxlength/minlength. It would be straightforward in the future to mitigate parser issues by adding options to specify a finite limit on the integer length and decimal length of a number grammar. The ideal length limit will likely vary widely per customer or application - and even more so why an API-powered grammar builder (achem, ruby_speech) is well-suited to solving this problem rather than a global and untouchable builtin.To be fair, LumenVox is not the only vendor who recommends/steers toward a finite limit on a number sequence. As seen in the Voxeo docs:
number - Specifies a voice or DTMF input grammar that recognizes numeric input. For Prophecy ASR, the maximum length of detectable digits is 16 when using this built-in grammar.
side note: Rebasing just to make sure that tests still pass following the nokogiri 1.7.0 release. No recent changes to this PR.
Rebasing in order to get an accurate code coverage delta.
@benlangfeld What are your thoughts on this PR? Does my reasoning sound legit? 😁
@benlangfeld Is it okay to merge this?
Thank you!
Make the number DTMF grammar more performant for LumenVox. Note that the more re-use we employ with rulerefs, the fewer cycles that their FST has to create.
Opening this for peer review along with the open source community.