Make parse results deterministic across runs#66
Merged
Conversation
Repeated parses of the same sentence could yield different trees whenever the parse forest contained subtrees with exactly equal reduction scores. The root cause was State::getHash() in eparser.cpp XOR-ing raw heap pointers (m_pProd, m_pw) into the hash: the Column hash-bin enumeration order then varied with malloc addresses, changing the order of addFamily() calls when building the SPPF, and thereby which equal-score family the reducer's index-based tie-breaker picked. - eparser.h/eparser.cpp: make State::getHash() content-based, using Production::getId() and a new Label::getHash()/Node::getHash() (nonterminal index, dot, production id, token span) instead of pointer values. State equality is unchanged. - grammar.py: hash Terminal and Nonterminal by their creation-order sequence number instead of id(), so that set/dict iteration order over grammar items is stable across processes and PYTHONHASHSEED values. The cached _hash snapshot is kept separate from _index, which is renumbered after the grammar is read. - test_parse.py: add test_deterministic_reduction with sentences that previously flipped between equal-score parses (verified to fail against the old parser core). - test_native_matching.py: update a comment that documented the old nondeterministic tie-breaking. Parse results are now stable across repeated parses, processes and hash seeds. Note that this can change which of two equally-scored parses is returned, compared to earlier versions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Repeated parses of the same sentence could yield different trees whenever the parse forest contained subtrees with exactly equal reduction scores — the tie was effectively broken by malloc addresses. This was a long-standing issue, previously worked around in tests by comparing forest combination counts instead of reduced trees.
Root cause
State::getHash()ineparser.cppXOR-ed raw heap pointers (m_pProd,m_pw) into the hash. Since aColumnstores Earley states in hash bins andColumn::nextState()enumerates them bin by bin, the state processing order varied with allocation addresses between runs. That changed the order ofaddFamily()calls when building the SPPF, and thereby the family indices used by the reducer's "lowest index wins" tie-breaker inreducer.py.Changes
eparser.h/eparser.cpp:State::getHash()is now content-based, usingProduction::getId()and a newLabel::getHash()/Node::getHash()(nonterminal index, dot, production id, token span) instead of pointer values. State equality is unchanged; only bin placement needed determinism.grammar.py:Terminal.__hash__andNonterminal.__hash__now use the creation-order sequence number instead ofid(), so set/dict iteration order over grammar items is stable across processes andPYTHONHASHSEEDvalues. The cached_hashsnapshot stays separate from_index, which is renumbered after the grammar is read.test_parse.py: newtest_deterministic_reductionwith sentences that previously flipped between equal-score parses; verified to fail against the old parser core.test_native_matching.py: updated a comment documenting the old nondeterministic tie-breaking.Verification
PYTHONHASHSEEDvalues._eparserbuild and passes with this change.Note
This can change which of two equally-scored parses is returned compared to 3.7.1 — stably, from now on. Worth a mention in the release notes of the next release.
🤖 Generated with Claude Code