-
Notifications
You must be signed in to change notification settings - Fork 85
Proof encoding in egglog itself #770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging this PR will degrade performance by 9.91%Comparing Summary
Performance Changes
Footnotes
|
fa46459 to
e484b95
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces proof generation capabilities to egglog by instrumenting the term encoding with proof tracking. The changes enable proofs to be generated and verified for equality reasoning in egglog programs.
Changes:
- Refactors TermDag API to use TermIds instead of Terms for cleaner ownership semantics
- Adds comprehensive proof infrastructure including encoding, format, checker, and extraction
- Implements primitive validators enabling automatic proof generation for built-in operations
Reviewed changes
Copilot reviewed 64 out of 64 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Adds proof mode support, ProveExists command, and integrates proof pipeline |
| src/typechecking.rs | Adds PrimitiveValidator type and support for primitives with validators |
| src/termdag.rs | Refactors API from Term to TermId with new pretty-printing support |
| src/proofs/* | New proof infrastructure including format, checker, encoding, extraction |
| src/sort/*.rs | Updates all sort implementations to use TermId API |
| src/prelude.rs | Adds LiteralConvertible trait for validator generation |
| tests/* | Adds validator tests and proof test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const MAX_PRETTY_LINE_WIDTH: usize = 80; | ||
| const PRETTY_INDENT_STEP: usize = 2; | ||
| const MIN_SHARED_TERM_SIZE: usize = 4; |
Copilot
AI
Jan 14, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These constants control pretty-printing behavior but lack documentation explaining their purpose and rationale. Consider adding comments explaining why these specific values were chosen.
13549f2 to
fe0fc44
Compare
| /// add_literal_prim!(eg, "<" = |a: i64, b: i64| -> bool { a < b }); | ||
| /// ``` | ||
| #[proc_macro] | ||
| pub fn add_literal_prim(input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm bad a writing macros, this one is AI generated so take care.
The rest of the code in this PR is mostly hand written
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am curious what a validator really looks like... kinda hard to review without that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See validator_test for an example (:
ezrosent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First round of comments.
| /// add_literal_prim!(eg, "<" = |a: i64, b: i64| -> bool { a < b }); | ||
| /// ``` | ||
| #[proc_macro] | ||
| pub fn add_literal_prim(input: TokenStream) -> TokenStream { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I am curious what a validator really looks like... kinda hard to review without that.
tests/merge-saturates.egg
Outdated
|
|
||
| (run 100) | ||
| (check (baz 50)) No newline at end of file | ||
| ;; TODO now that relations are desugared, this trick also doesn't work: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the issue? (Could be a good topic for a regular meeting if this hasn't already come up?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good topic for regular meeting. It doesn't work because the relation has a fresh sort, so you can't put it in the merge function position.
|
Looks like the performance regressions on |
fix up symbol gen better names fixing things fix header small fix working on justification nits doing a lot of refactoring small tweaks snapshots getting rid of justification, just proof working on encoding small example running again renames term tests passing again working on relation small fixes small docs proof normal form untested rule proof instrumentation proofs failing eqsat basic making progress proofs passing some tests small bug fix fix bug with to ast fix another bug fix term encoding again tests pass fmt working on prove query working prove command refactor to proofs folders working on format working on proof snapshots first proof snapshot working on bug with proof extraction found bug in proof productioN fixed a bug fix up detection for supporting proofs fix supported proofs subsume fix working on better printing working on printing again, spotted substitution bug wrote unification code, getting error remove debugging unify seems to be working more proof simplification proof simplifier commit push symmetry down working on != proofs supported list validator test working on validators validators better validators use string new tests working on making checker more accurate fix checking for functions refactor validators run validators better proof checker workingon checker simplification breaking something refactor working on checker refactor use prim validators small fix add todo working on bugs most proofs being checked big termid refactor error in add prim more refactor merge fn proof fix check merge proofs working on fixing bugs all tests passing fix simplification refactor simplifications name hints nits more docs docs improvement snap working on encoding working on docs better docs more docs more cleanup fix up Update src/proofs/proof_checker.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Update src/proofs/proof_checker.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> small nits
633d98d to
19873c4
Compare
| expression: snapshot_content | ||
| --- | ||
| ( | ||
| (Config (Cons 1 (Cons 2 (Cons 3 (e)))) (e) (e) 0) -> (Config (Cons 1 (Cons 2 (Cons 3 (e)))) (e) (e) 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #797
Now that relations are constructors, we get this
70db85d to
85acdc3
Compare
85acdc3 to
80031f9
Compare
ezrosent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also see if you can track down the codspeed slowdowns... but they're small enough that they could be within the noise threshold.
| a term table that stores the actual terms, and a view table storing representative terms along with their e-class (stored as the leader term). | ||
| The term encoding enables proof tracking, done at the | ||
| same time in this file. | ||
| The encoding keeps the operational semantics equivalent to the standard encoding (for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think OS + some observational equivalence relation would be fine here too... but then I think we should say something informal here that conveys that given that we don't have an "official" operational semantics for egglog. How about:
The encoding does not change any user-observable behavior of an egglog program, except for commands that let you 'pop the hood' like
print-function.
| "python_array_optimize", | ||
| "stresstest_large_expr", | ||
| ]; | ||
| // in parallel mode always skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead, I'd recommend exposing a function from core-relations that checked rayon::current_num_threads and such. We can still run things in parallel when it's a release build. And if we ever changed the debug_assertions bit we won't have to hunt down every file that asked about it (most of the heuristics for parallelism are in one file I think).
This PR
--proofsflag.provecommand.In the future we'll want
proveto prove equivalence.