Skip to content

Fix(Processes): Make subterm hoisting deterministic across processes - #422

Merged
saulshanabrook merged 3 commits into
egraphs-good:mainfrom
wjrforcyber:fix/deterministic-subterm-hoisting
Sep 18, 2026
Merged

saulshanabrook merged 3 commits into
egraphs-good:mainfrom
wjrforcyber:fix/deterministic-subterm-hoisting

Conversation

@wjrforcyber

@wjrforcyber wjrforcyber commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Issue

Shared-subexpression discovery used set.pop(), so unspecified hash-table order—affected by Python's per-process hash randomization—could change which repeated nodes were discovered and hoisted first. This changed synthetic let numbering and serialized e-graph output across otherwise identical processes.

Fix

After integrating current main, _exprs_multiple_parents() uses deterministic ordered DAG traversal with explicit incoming-edge counts, preserving source order and returning each shared node once.

Test

The regression runs the same high-level program in fresh subprocesses with fixed PYTHONHASHSEED values 1, 2, and 3, then asserts that the public saved Egglog transcripts match. The previous set-based traversal produces different transcripts for these seeds.

Changelog

Adds an UNRELEASED entry linking #422.

@codspeed

codspeed Bot commented Sep 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 14 untouched benchmarks


Comparing wjrforcyber:fix/deterministic-subterm-hoisting (20c7990) with main (8dd2434)

Open in CodSpeed

@saulshanabrook

Copy link
Copy Markdown
Member

Thank you for this!

@saulshanabrook

Copy link
Copy Markdown
Member

@actions-user changelog

@saulshanabrook

Copy link
Copy Markdown
Member

This was fixed in my other branch that integrates a bunch of upstream changes, but keeping this test case, thank you!

@saulshanabrook
saulshanabrook merged commit af2c87a into egraphs-good:main Sep 18, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants