8000 Fix unsat cache in equivalence checking by blishko · Pull Request #747 · ethereum/hevm · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix unsat cache in equivalence checking #747

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

8000
Merged
merged 1 commit into from
May 19, 2025
Merged

Fix unsat cache in equivalence checking #747

merged 1 commit into from
May 19, 2025

Conversation

blishko
Copy link
Collaborator
@blishko blishko commented May 16, 2025

Recently, in a refactoring, the write to the cache disappeared, so the cache was actually always empty.
Here we bring back writing to the cache after unsat result.

Moreover, we make additional changes related to the cache: Given a set of proposition, we check if it is a superset of any known unsatisfiable set of propositions. The previous check for subset relation was not correct: a subset of an unsatiafiable set can be satisfiable.
Related to that, we order the queries from the smallest to largest, so that we increase the likelyhood of cache hits.

Description

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

Recently, in a refactoring, the write to the cache disappeared, so the
cache was actually always empty.
Here we bring back writing to the cache after unsat result.

Moreover, we make additional changes related to the cache:
Given a set of proposition, we check if it is a *superset* of any
known unsatisfiable set of propositions. The previous check for subset
relation was not correct: a subset of an unsatiafiable set can be
satisfiable.
Related to that, we order the queries from the smallest to largest, so
that we increase the likelyhood of cache hits.
@msooseth
Copy link
Collaborator

Oh wow, yes. Definitely. It's a superset. And the ordering via compare is much bette 8000 r. These are actually the right things.... I think it's good, although I would like to integrate them into #743 and merge that way.

I double-checked the superset and indeed it's what we want:

ghci> supersetAny a bs = any (`isSubsetOf` a) bs
ghci> supersetAny (fromList [5]) [fromList [5,6]]
True

and the new compare is also correct:

ghci> sortBySize = sortBy (\(a, _) (b, _) -> compare (Set.size a) (Set.size b))
ghci> sortBySize [(fromList [5,6],"a"), (fromList [4], "a")]
[(fromList [4],"a"),(fromList [5,6],"a")]

We should do the small ones first, then the supersets will hit more often.

I am thinking about merging and rebasing my PR on top of this. What do you think?

@blishko blishko marked this pull request as ready for review May 19, 2025 08:57
Copy link
Collaborator
@msooseth msooseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks!!

@msooseth msooseth merged commit 3d41c32 into main May 19, 2025
9 checks passed
@msooseth msooseth deleted the fix-unsat-cache branch May 19, 2025 09:27
@msooseth msooseth mentioned this pull request May 26, 2025
4 tasks
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
0