Skip to content
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

Merge our unification and matching algorithms #3785

Closed
goodlyrottenapple opened this issue Apr 8, 2024 · 0 comments · Fixed by #3810 · May be fixed by runtimeverification/hs-backend-booster#573
Closed

Merge our unification and matching algorithms #3785

goodlyrottenapple opened this issue Apr 8, 2024 · 0 comments · Fixed by #3810 · May be fixed by runtimeverification/hs-backend-booster#573
Assignees
Labels
runtimeverification/hs-backend-booster Issues transferred from runtimeverification/hs-backend-booster

Comments

@goodlyrottenapple
Copy link
Contributor

At the moment we have essentially two copies of the "unification" algorithm in the booster:

https://github.com/runtimeverification/hs-backend-booster/blob/754e6ca645d96b0460355a4d61bdf42b3b4f6ff2/library/Booster/Pattern/Unify.hs#L172
https://github.com/runtimeverification/hs-backend-booster/blob/754e6ca645d96b0460355a4d61bdf42b3b4f6ff2/library/Booster/Pattern/Match.hs#L120

However, I would argue that our "unification" when applying rewrite rules is just a slightly different version of the matching algorithm we use for applying function and simplification rules. Namely, we expressly forbid any overlap of variables in the two terms and expressly check at the end of the unification algorithm that we in fact obtained a matching substitution, such that we have subst(rule_LHS) = current_configuration. Since there is no significant algorithmic difference between our "unification" and matching (match1 is a copy of unify1 with various tweaks), it would be better to merge the two as it will be much easier to see what exactly the differences between the two algorithms are. E.g. match1 allows matching two terms f1(...) where f1 isn't a constructor, whereas unify1 does not allow this. Conversely, unify1 works for a subset of map/set/list problems, whereas support for these is lacking in match1.

This issue has been raised before, but is more pertinent now that we are tackling the implication endpoint in the booster, which requires tweaks to either versions to e.g. lift the restriction that the variables in the two terms are completely disjoint. Instead of making yet another copy of essentially the same code, we should instead merge unify1/match1 and put the differences between rule/function/implication matching in a single place, such that it is easy to see what the subtle differences (if any) are between these versions.

@goodlyrottenapple goodlyrottenapple self-assigned this Apr 8, 2024
@Baltoli Baltoli added the runtimeverification/hs-backend-booster Issues transferred from runtimeverification/hs-backend-booster label Apr 11, 2024
@Baltoli Baltoli transferred this issue from runtimeverification/hs-backend-booster Apr 11, 2024
goodlyrottenapple added a commit that referenced this issue Apr 15, 2024
Fixes #3785 by merging the two versions of our matching algorithm, one
used in matching rewrite rules and another matching
function/simplification rules. This is a first pass which mostly just
merges the two algorithms, prserving most of the divergent behaviour
under the `Rewrite`/`Eval` `MatchType` flag. A followup refactor should
clean up this code further.

This PR is a port of
runtimeverification/hs-backend-booster#573 where
the development and testing happened.

---------

Co-authored-by: github-actions <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtimeverification/hs-backend-booster Issues transferred from runtimeverification/hs-backend-booster
Projects
None yet
2 participants