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 <noreply@anthropic.com>

Author: vthorsteinsson

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
       {

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*);

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

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:

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