Enhancement/refactor grammar encoding#99
Conversation
|
I tested my changes against the old parser to ensure full compatibility. To accomplish this, I compared the results of processing all EBNF files in the repository: def test_refactor_parser(self):
# get all the ebnf files in examples/grammars and subdirectories
import os
from transformers_cfg.old_parser import parse_ebnf as old_parse_ebnf
files = []
for root, _, filenames in os.walk("examples/grammars"):
for filename in filenames:
if filename.endswith(".ebnf"):
files.append(os.path.join(root, filename))
for file in files:
with open(file, "r") as f:
src = f.read()
old_state = old_parse_ebnf(src)
new_state = parse_ebnf(src)
self.assertListEqual(old_state.grammar_encoding, new_state.grammar_encoding, f"The grammar encoding of {file} is different")Here the |
|
|
||
|
|
||
| @dataclass | ||
| class TerminatedElement(GrammarElement): |
There was a problem hiding this comment.
Maybe the the name TerminalElement would be more appropriate as terminal is the word used in grammar theory
There was a problem hiding this comment.
Just to see if I understand correctly, this TerminalElement is used to match a single char, isn't it ?
There was a problem hiding this comment.
The TerminalElement are all the elements that are not References. So it's just list of ranges. For a single char, it will be a list with one range: [(ord(char), ord(char)].
|
|
||
| @dataclass | ||
| class ReferenceElement(GrammarElement): | ||
| reference_id: int |
There was a problem hiding this comment.
Maybe the name referee_id would be more explictly expressing that this is the symbol id that is referred to
| return outbuf | ||
|
|
||
|
|
||
| class ParseState: |
There was a problem hiding this comment.
I start to realise that ParseState was really a bad name... How do you think CompiledGrammar?
There was a problem hiding this comment.
After thinking about it, I think the ParseState is a good name because it represents the current parse state when parsing the grammar. But at the end, we can indeed use CompiledGrammar.
|
I’ll need a bit more time to review this PR—I’ve kind of forgotten how the parser works 😅. |
2 participants
This PR refactors the parser logic by introducing multiple classes to represent different parts of the grammar:
GrammarElement: An abstract class representing a single element. It has two subclasses:TerminatedElement: Represents literals, ranges, and similar constructs.ReferenceElement: Represents references, with a single attribute:reference_id.AlternativeElements: A class representing an alternative, containing a list ofsymbolsthat is itself a list of GrammarElementobjects. Thesymbolsare useful for repetition operators that apply not just on the previousGrammarElement` but on the entire previous symbol.GrammarRule: A class representing a rule, containing a list ofAlternativeElementsobjects.Each of these classes implements the
Codableinterface and provides an implementation of theserializemethod:The
serializemethod generates a list of integers, maintaining consistency with the originalgrammar_encodinglist and the rest of the codebase.I am currently trying to add a
graphmethod to theParseStateclass to be able to create nice graphics to visualise the grammar.Update: The
graphmethod is now fully functional.