From 1e25ca6fc4dadb3d6ed3c433561bc7e7278b89b1 Mon Sep 17 00:00:00 2001 From: Vilhjalmur Thorsteinsson Date: Tue, 14 Jul 2026 11:17:39 +0000 Subject: [PATCH 1/2] Make parse results deterministic across runs 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 --- src/reynir/eparser.cpp | 16 ++++++++++++---- src/reynir/eparser.h | 18 ++++++++++++++++++ src/reynir/grammar.py | 20 ++++++++++++++------ test/test_native_matching.py | 9 +++++---- test/test_parse.py | 23 +++++++++++++++++++++++ 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/reynir/eparser.cpp b/src/reynir/eparser.cpp index 83d0d62..fcc9bdf 100644 --- a/src/reynir/eparser.cpp +++ b/src/reynir/eparser.cpp @@ -115,10 +115,18 @@ friend class AllocReporter; UINT getHash(void) const { - return ((UINT)this->m_iNt) ^ - ((UINT)((uintptr_t)this->m_pProd) & 0xFFFFFFFF) ^ - (this->m_nDot << 7) ^ (this->m_nStart << 9) ^ - (((UINT)((uintptr_t)this->m_pw) & 0xFFFFFFFF) << 1); + // Content-based hash: this must not depend on memory + // addresses, since the hash determines the order in which + // states are enumerated from a Column's hash bins, which + // in turn determines the order of families of children + // in the resulting parse forest. Using pointer values here + // would make parse results nondeterministic between runs + // whenever the reducer encounters exact score ties. + UINT h = ((UINT)this->m_iNt) ^ + (this->m_nDot << 7) ^ (this->m_nStart << 9); + h = h * 31 + (this->m_pProd ? this->m_pProd->getId() : (UINT)-1); + h = h * 31 + (this->m_pw ? this->m_pw->getHash() : 0); + return h; } BOOL operator==(const State& other) const { diff --git a/src/reynir/eparser.h b/src/reynir/eparser.h index a9b5ae8..bc74f52 100644 --- a/src/reynir/eparser.h +++ b/src/reynir/eparser.h @@ -270,6 +270,20 @@ class Label { this->m_nJ == other.m_nJ; } + // Content-based hash, deliberately independent of memory + // addresses (the production is represented by its id, not + // its pointer), so that hash-derived orderings are stable + // between runs + UINT getHash(void) const + { + UINT h = (UINT)this->m_iNt; + h = h * 31 + this->m_nDot; + h = h * 31 + (this->m_pProd ? this->m_pProd->getId() : (UINT)-1); + h = h * 31 + this->m_nI; + h = h * 31 + this->m_nJ; + return h; + } + }; @@ -310,6 +324,10 @@ class Node { BOOL hasLabel(const Label& label) const { return this->m_label == label; } + // Deterministic content-based hash of this node's label + UINT getHash(void) const + { return this->m_label.getHash(); } + void dump(Grammar*); static UINT numCombinations(Node*); diff --git a/src/reynir/grammar.py b/src/reynir/grammar.py index 083fa25..f0e6bfe 100644 --- a/src/reynir/grammar.py +++ b/src/reynir/grammar.py @@ -178,12 +178,15 @@ def __init__(self, name: str, fname: Optional[str] = None, line: int = 0) -> Non # Give all nonterminals a unique, negative sequence number for hashing purposes self._index = Nonterminal._index Nonterminal._index -= 1 - self._hash = id(self).__hash__() + # Use the creation-order sequence number as the hash. It is + # deterministic between runs (unlike id()), so that iteration + # order over sets and dicts of nonterminals is stable, and it + # never changes during the object's lifetime (unlike self._index, + # which may be renumbered after the grammar has been processed). + self._hash = self._index def __hash__(self) -> int: - """Use the id of this nonterminal as a basis for the hash""" - # The index may change after the entire grammar has been - # read and processed; therefore it is not suitable for hashing + """Return the cached, deterministic hash of this nonterminal""" return self._hash def __eq__(self, o: Any) -> bool: @@ -274,8 +277,13 @@ def __init__(self, name: str) -> None: self._name = name self._index = Terminal._index Terminal._index += 1 - # The hash is used quite often so it is worth caching - self._hash = id(self).__hash__() + # The hash is used quite often so it is worth caching. + # Use the creation-order sequence number: it is deterministic + # between runs (unlike id()), so that iteration order over sets + # and dicts of terminals is stable, and it never changes during + # the object's lifetime (unlike self._index, which may be + # renumbered after the grammar has been processed). + self._hash = self._index def __hash__(self) -> int: return self._hash diff --git a/test/test_native_matching.py b/test/test_native_matching.py index 04911df..1e0ac81 100644 --- a/test/test_native_matching.py +++ b/test/test_native_matching.py @@ -131,10 +131,11 @@ def _parse_results(disable_native: bool): """Parse the corpus with a fresh parser, native matching on or off. Returns, per sentence, the number of parse tree combinations in the - forest (or None if the sentence did not parse). Note that we compare - forest sizes rather than reduced trees, since the reducer may break - exact score ties differently between runs; the forest itself is - fully determined by the token/terminal match results.""" + forest (or None if the sentence did not parse). We compare forest + sizes since they are fully determined by the token/terminal match + results, which is exactly what this module tests. (Reduced trees + are nowadays deterministic as well; see test_parse.py:: + test_deterministic_reduction.)""" key = "GREYNIR_DISABLE_CPP_MATCHING" old = os.environ.get(key) try: diff --git a/test/test_parse.py b/test/test_parse.py index 0fa1a4e..707d33c 100644 --- a/test/test_parse.py +++ b/test/test_parse.py @@ -2209,6 +2209,29 @@ def test_þau(r): # assert s.tree.S.IP.NP_SUBJ.PP.NP.tidy_text == "þeim Gunnlaugi" +def test_deterministic_reduction(r): + """Repeated parses of the same sentence must yield the same tree, + even when the parse forest contains subtrees with exactly equal + scores. The ambiguous sentences below used to come out differently + between runs when the C++ parser's Earley state hash included + memory addresses, making the family order in the forest - and + thereby the reducer's tie-breaking - nondeterministic.""" + sentences = [ + "Ása sá sól.", + "Konan sem kom í heimsókn í gær ætlar að kaupa nýja íbúð í miðbænum.", + "Hr. Jón Jónsson býr á Laugavegi 26 og á 3,4 milljónir króna í banka.", + "Það rignir sjaldan í Reykjavík í júlí en þó gerist það stundum.", + "Tuttugu og þrír hestar, fimm kýr og tólf kindur voru á bænum.", + ] + for sent in sentences: + flats = set() + for _ in range(5): + s = r.parse_single(sent) + assert s is not None and s.tree is not None + flats.add(s.tree.flat) + assert len(flats) == 1, f"Nondeterministic parse of '{sent}': {flats}" + + def test_aukafall(r): s = r.parse_single("Mér blöskrar framkoma Páls.") assert s and s.tree From f2c13c3ce6ca99a426f87e9a27bfc2d5f7353752 Mon Sep 17 00:00:00 2001 From: Vilhjalmur Thorsteinsson Date: Tue, 14 Jul 2026 11:25:46 +0000 Subject: [PATCH 2/2] Bump version to 3.7.2 Co-Authored-By: Claude Fable 5 --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2798f5b..8a56650 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "reynir" -version = "3.7.1" +version = "3.7.2" description = "A natural language parser for Icelandic" authors = [{ name = "Miðeind ehf.", email = "mideind@mideind.is" }] maintainers = [{ name = "Miðeind ehf.", email = "mideind@mideind.is" }] diff --git a/uv.lock b/uv.lock index d1fdaad..9b33966 100644 --- a/uv.lock +++ b/uv.lock @@ -409,7 +409,7 @@ wheels = [ [[package]] name = "reynir" -version = "3.7.1" +version = "3.7.2" source = { editable = "." } dependencies = [ { name = "cffi" },