-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
Bug: GBNF repetition rewrite results in unsupported left recursion #7572
Comments
That is a deliciously complex grammar in your example, btw. I love this as a stress-test, thank you!
I'm left wondering what in the world might be meant by There are many ways to think about the dangers / problems of left-recursion, but one way I like to think about it is that left-recursion occurs when the grammar engine "ticks" forward without making any real progress. So long as there is a path that exists for this, then the grammar engine can process infinitely without making any headway, and thus the engine "hangs". Before #7083, that's exactly what would happen -- the grammar engine would simply hang until it segfaulted from a lack of memory. So the fact that it alerts you to the problem of left-recursion is a nice improvement from where we were a few weeks ago -- that's encouraging! :)
Perhaps, yes. It sounds like what you're asking for is that if there is a grammar segment that is zero-length, then it be auto-removed from the tree. I need to think through the implications of this, and see if there are other ways that this can get us into trouble. Are there ways that this can come back to bite us? I need to think through this... In the meantime, instead of replacing |
@HanClinto Further down in the gist https://gist.github.com/hoehrmann/f234c1156ee5ef7b24cb589c14aaefda?permalink_comment_id=5070397#gistcomment-5070397 is a variant where I removed the redundant empty string alternative. llamap.cpp then goes into SIGSEGV after a couple of lines (or pretty much immediately if you change the root to the |
Confirmed, thank you! I ran the following command:
And it crashed after:
I will look into that issue -- not sure if we want to open that as a new issue or not. In the meantime, do you have any other thoughts about grammars with |
(This issue is about turning repetition into left recursion and then complaining that the generated left recursion is not supported. That llama.cpp runs out of stack space for the other example is a separate issue, I think.) (This is probably not what you are looking for, but since you asked:) Well, I can add this from a document I am working on. This turns a CFG into a graph with nodes that are labeled with one of empty string, a terminal (or a character class), or a stack label (acting as a stack symbol, moving over Grammar graph constructionRuleA = B ; named rule A is defined as B graph LR
start --> startRule;
startRule["`start id(A)`"] --> B;
B --> finalRule["`final id(A)`"];
finalRule --> final;
Empty string"" ; zero terminals graph LR
start --> e["` `"];
e --> final;
Terminal"a" ; single character "a" (or "A") graph LR
start --> term;
term["`terminal a`"];
term --> final;
Non-terminal referenceA ; reference to named rule A graph TB
start --> startRef;
startRef["`start ref id(A)`"] --> startA["`start A`"];
startA --> elision[ ... ];
elision --> finalA["`final A`"];
finalA --> finalRef["`final ref id(A)`"];
finalRef --> final;
References to a non-terminal named rule connect to the start and final nodes for the corresponding rule definition. This adds edges to the nodes of the rule definition. The Strings"example" ; e followed by x followed by ... Strings are sequences of terminals. SequenceA B ; A followed by B graph LR
start --> A;
A --> B;
B --> final;
One-or-more1*<A> ; one or more A graph LR
start --> enter[ ];
enter --> A;
A --> exit[ ];
exit --> enter;
exit --> final;
Zero-or-more is a choice between the empty string and one or more of a pattern. Other repetition patterns can be spelled out explicitly. For instance, optional patterns are a choice between the pattern and the empty string, and if a pattern needs to appear four times, the pattern can be repeated four times in the grammar. Unordered ChoiceA / B ; A or B or both graph TD
start --> enter[ ];
enter --> A[A];
enter --> B[B];
A --> exit[ ];
B --> exit[ ];
exit --> final;
|
(The grammar graph does not use recursion to represent infinite repetition; for actual recursion, a pragmatic approach would be to let users specify a depth limit for the grammar. Replacing a recursive rule |
Thank you for the writeup and response!
I think you're right -- this particular issue is about the way that we complain about not supporting left-recursion rather than handling it more gracefully. My vote would be that you open the running-out-of-stack-space issue as a separate one, and repost your grammar / reproduction steps in there. I suspect it may eventually get merged in with one of the other grammar improvement issues that's already ongoing, but it would be nice to have it broken out regardless.
It's a good read, thank you!
This is really cool -- nice job on all of the Mermaid graphs! For a while I was starting down the road of trying to add Mermaid graph generation to the gbnf-validator program (in order to better help visualize / debug grammars), and you have inspired me to take another swing at that. :)
Yes, I suspect that you're correct. I've looked at several alternative methods for handling left-recursion, but they often require such a fundamental shift in the way that one "hangs" the problem and approach to grammar. There are a number of particulars related to such piecemeal token generation that makes it very expensive to scan forward-and-backward in the text stream. The way that we recursively build our grammar trees and tick them all forward one small set of characters at a time works so nicely for the way that LLMs generate text. Because our grammar engine maintains separate trees for every possible pathway simultaneously, we don't require much lookahead (at most, a token's worth of characters into the future), and we never have to resample because we went down a bad pathway (the |
BTW, I really love the grammar tree application visualization that's in this documentation. Thank you for linking it!
Yet another thing that I would love to emulate in our gbnf-validator program. |
This issue was closed because it has been inactive for 14 days since being marked as stale. |
What happened?
Replacing the "FIXME" strings in https://gist.github.com/hoehrmann/f234c1156ee5ef7b24cb589c14aaefda?permalink_comment_id=5070379#gistcomment-5070379 by the empty string makes llama.cpp see left recursion that is not in the original grammar, presumably due to how it rewrites repetition into recursion. The basic construct is
In case of the grammar above, the empty alternative does not add anything to the grammar since it is under a zero-or-more repetition anyway. It would be nice if the grammar rewriter would lift the empty string out of the alternatives and adjust the quantifier if necessary before rewriting this into recursion.
Name and Version
version: 2891 (9a17ab9)
built with cc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 for x86_64-linux-gnu
What operating system are you seeing the problem on?
Linux
Relevant log output
The text was updated successfully, but these errors were encountered: