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

whisper grammar: experimental implementation with boost::spirit #2127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fraxy-v
Copy link
Contributor

@fraxy-v fraxy-v commented May 7, 2024

An explorative re-implementation of @ejones's parser which IMHO captures the advantages and disadvantages of boost::spirit very well.

@ulatekh
Copy link
Contributor

ulatekh commented Jun 4, 2024

I'm interested in alternate grammar implementations, since the existing one has odd accuracy issues, especially when it comes to matching whole words from the grammar. But what does this branch do? It doesn't seem to substitute the boost::spirit implementation in place of the existing grammar implementation.

@fraxy-v
Copy link
Contributor Author

fraxy-v commented Jun 5, 2024

True. It doesn't replace the original, but adds a test_parse method and its implementation in test-grammar-parser.cpp, and a bit of a hand-wavy test test-grammar.cpp which compares the new boost::spirit implementation against the old one. That's the raw stage that I left it in, since I just wanted to see how it would work with boost::spirit.

I guess you can just plug an example input exhibiting these issues into the test code and see if boost::spirit behaves differently and, if it doesn't, if it can be easily fixed. Let me know if you need help with that.

@fraxy-v
Copy link
Contributor Author

fraxy-v commented Jun 5, 2024

Having said that, I would also say it may be better to fix the problem in the original implementation. I am fairly familiar with the grammar parser, so If you give me an example grammar input demonstrating the oddities, I may be able to suggest a fix.

@ulatekh
Copy link
Contributor

ulatekh commented Jun 5, 2024

The only grammar I have is a very large one. If I can figure out how to cut it down to where it reproduces the problem, I will. I imagine the problem is that whisper.cpp's grammar is character-based, and if it finds multiple possibilities that begin with the same prefix, but can't decide between them, it terminates recognition at the common prefix, instead of realizing that an incomplete word is not a match.

@fraxy-v
Copy link
Contributor Author

fraxy-v commented Jun 6, 2024

Okay, but that doesn't sound like a parser bug, unless you have some modification in mind.

@ulatekh
Copy link
Contributor

ulatekh commented Jun 6, 2024

It's a bug in how whisper.cpp applies grammars, though. It does me no good to get a response back from Whisper that doesn't match the grammar I gave it.

@fraxy-v
Copy link
Contributor Author

fraxy-v commented Jun 7, 2024

Fair enough, but the application of the grammar is terra incognita for me. Want to help, but I don't know if I could. @ggerganov what do you think?

@ggerganov
Copy link
Owner

A repro would be needed - without a specific example of the problem, I don't think we can do anything

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

Successfully merging this pull request may close these issues.

3 participants