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

Add macro rule for capture avoidance in beta redexes and its elaboration #11513

Merged
merged 20 commits into from
Jan 21, 2025

Conversation

ajreynol
Copy link
Member

@ajreynol ajreynol commented Jan 10, 2025

This adds a macro rewrite to capture some of the rewrites in the UF rewriter and adds proof elaboration for it, which is expected to be accomplished by (possibly multiple steps of) alpha equivalence.

Note that it uses a more elegant solution than #11329, which lifted APPLY_UF to higher-order applications to ensure shadowing was eliminated. Now, the rewriting the APPLY_UF is done "in-place", i.e. (APPLY_UF L ts) ---> (APPLY_UF L' ts). As a result, we revert to a behavior where HO_APPLY is never introduced in first order logics, making the change to TheoryUF unecessary. It also means we don't rely on higher-order beta-reduction in first-order logics. This reenables alethe on these regressions.

This rewrite is used to ensure that beta reduction does not introduce capture avoiding substitutions. The UF rewriter is refactored to use the reference implementation.

Adds a new utility method (getMatchConds) to node_algorith.h which will be used for a few other pending tasks in proof elaboration.

Fills 1 proof hole in our regressions and 287 proof holes on SMT-LIB.

@ajreynol ajreynol added normal Priority moderate Complexity labels Jan 10, 2025
@ajreynol ajreynol requested a review from HanielB January 10, 2025 21:35
@ajreynol ajreynol added major Priority and removed normal Priority labels Jan 14, 2025
@ajreynol ajreynol added the do not merge Do not merge this PR if you are not the author label Jan 14, 2025
@ajreynol
Copy link
Member Author

Found an issue in the elaboration that leads to proof holes, will need to fix.

github-merge-queue bot pushed a commit that referenced this pull request Jan 15, 2025
This ensures that we apply top-level substitutions and rewrite bodies of
define-fun eagerly. This is required for
#11513.

In particular, currently, due to the fact that variables are *not*
fresh, and the rewriter does not rewrite inside of operators, beta
redexes could appear arbtirarily nested in APPLY_UF. As a result, we
were requiring the fairly sophisticated "shadow elimination" methods for
handling benchmarks such as:
```
(define-fun P ((x Int)) Bool false)
(define-fun Q ((x Int)) Bool (P x))
(assert (Q 0))
(check-sat)
```

In particular, `Q` would be defined to be the lambda `(lambda ((x Int))
(@ (lambda ((x Int)) false) x))`, which assuming the rewriter does not
rewrite inside of operators, leads us to requiring shadow elimination to
do beta reduction.

We previously made a nested call from within the rewriter to ensure such
shadowing was resolved prior to beta reduction. The PR
#11329 refactored this so that beta
redexes for non-rewritten lambdas were lifted to HO terms so that the
rewriter could track proofs without being self referential. However, as
a downside, we now require HO reasoning in first order logics, in
particular, lambdas could be rewritten and beta reduction could be done
for HO applications. The PR #11513
addresses this shortcoming by allowing non-rewritten lambdas to be
beta-reduced. However, proof elaboration fails since this requires
arbitrarily nested shadow elimination. In particular, a proof of the
above benchmark would (unintuitively) introduce shadowing elimination +
alpha equivalence.

The solution is to apply top-level substitutions + rewriting to *bodies*
of lambdas as they are defined. this prevents HO rewriting while
eliminating complications due to shadowing in beta redexes.

We do not rewrite defined constants, since this complication does not
arise and it better to be lazy based on some benchmarks in SMT-LIB
(cpachecker in QF_UFLRA).

There are several motivations for this change:
1. It makes the proofs significantly simpler and resolves all open
issues related to define-fun (after
#11513 is merged),
2. It makes the output of e.g. `-o post-asserts` much more intuitive. In
particular, prior to this commit, we'd get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool (@ (lambda ((x Int)) false) x))
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
whereas now we get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool false)
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
@ajreynol ajreynol removed the do not merge Do not merge this PR if you are not the author label Jan 15, 2025
include/cvc5/cvc5_proof_rule.h Outdated Show resolved Hide resolved
include/cvc5/cvc5_proof_rule.h Outdated Show resolved Hide resolved
src/expr/node_algorithm.h Outdated Show resolved Hide resolved
src/expr/node_algorithm.cpp Outdated Show resolved Hide resolved
src/expr/node_algorithm.cpp Show resolved Hide resolved
src/expr/node_algorithm.h Outdated Show resolved Hide resolved
include/cvc5/cvc5_proof_rule.h Outdated Show resolved Hide resolved
src/theory/uf/theory_uf_rewriter.cpp Outdated Show resolved Hide resolved
src/theory/uf/theory_uf_rewriter.cpp Outdated Show resolved Hide resolved
src/theory/uf/theory_uf_rewriter.cpp Show resolved Hide resolved
* where :math:`\sigma` replaces :math:`x_1, \dots, x_n` by
* :math:`y_1, \dots, y_n`. The terms may either be of kind
* `cvc5::Kind::APPLY_UF` or `cvc5::Kind::HO_APPLY`.
* This rule ensures that :math:`y_1, \dots, y_n` are not in the free
* variables of :math:`t_1 \ldots t_n` and
* :math:`(\lambda x_1 \ldots x_n.\> t)` is alpha-equivalent to
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it the case that the lambda expressions are necessarily gonna be alpha-equivalent when \sigma is a valid renaming, which is the consequence of the condition above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not if t had a variable in y_1 ... y_n that was not in x_1 ... x_n.

Copy link
Member

Choose a reason for hiding this comment

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

Well, t will not, since y1...yn are not among the free variables of the lambda expression. If you properly constrain the substitution you'll gen an alpha equivalent term, which is what you want.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is more consice to say they are alpha equivalent, which implies this condition as well as saying that y_1 ... y_n are unique.

Copy link
Member

@HanielB HanielB Jan 17, 2025

Choose a reason for hiding this comment

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

You do not need to say that they are alpha equivalent if you say the renaming is such that it does not include free variables from the lambda expression. You could say:

This rule requires that :math:`y_1, \dots, y_n` are pairwise distinct and not in the free variables of :math:`(\lambda x_1 \ldots x_n.\> t)`.

Copy link
Member Author

@ajreynol ajreynol Jan 17, 2025

Choose a reason for hiding this comment

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

It's not that simple, I've made the description more accurate now.

Examples of the rule:
(_ (lambda x. forall x. P(x)) a) ---> (_ (lambda y. forall x. P(x)) a).
(_ (lambda y. forall x. P(y)) (f x)) ---> (_ (lambda y. forall z. P(y)) (f x)).
(_ (lambda y. forall xu. P(y, u)) (f x)) ---> (_ (lambda y. forall zu. P(y, u)) (f x)).
(_ (lambda y. forall x. P(y) ^ forall x. Q(y)) (f x)) ---> (_ (lambda y. forall u. P(y) ^ forall v. Q(y)) (f x)).

Copy link
Member

@HanielB HanielB Jan 20, 2025

Choose a reason for hiding this comment

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

I see now. This rule is doing much more than I assumed it was. :) I like how you extended the documentation, thanks. I'd suggest to include an example, like the second one of the comment above.

And maybe we should consider renaming the rule, because that's what misled me: I had assumed from the name that you were just eliminating shadowing among the variables bound by the lambda, but you are in fact renaming variables in the lambda expression to avoid capture of the variables in t_1 ... t_n(if beta-reduction is done with a not capture-avoiding substitution, which is exactly what you want...).

Copy link
Member Author

Choose a reason for hiding this comment

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

MACRO_LAMBDA_CAPTURE_AVOID?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

include/cvc5/cvc5_proof_rule.h Outdated Show resolved Hide resolved
src/expr/node_algorithm.h Outdated Show resolved Hide resolved
@ajreynol ajreynol changed the title Add macro rule for lambda shadow elimination and its elaboration Add macro rule for capture avoidance in beta redexes and its elaboration Jan 21, 2025
@HanielB HanielB added this pull request to the merge queue Jan 21, 2025
Merged via the queue into cvc5:main with commit 0eb61b1 Jan 21, 2025
11 checks passed
HanielB pushed a commit to HanielB/cvc5 that referenced this pull request Jan 21, 2025
This ensures that we apply top-level substitutions and rewrite bodies of
define-fun eagerly. This is required for
cvc5#11513.

In particular, currently, due to the fact that variables are *not*
fresh, and the rewriter does not rewrite inside of operators, beta
redexes could appear arbtirarily nested in APPLY_UF. As a result, we
were requiring the fairly sophisticated "shadow elimination" methods for
handling benchmarks such as:
```
(define-fun P ((x Int)) Bool false)
(define-fun Q ((x Int)) Bool (P x))
(assert (Q 0))
(check-sat)
```

In particular, `Q` would be defined to be the lambda `(lambda ((x Int))
(@ (lambda ((x Int)) false) x))`, which assuming the rewriter does not
rewrite inside of operators, leads us to requiring shadow elimination to
do beta reduction.

We previously made a nested call from within the rewriter to ensure such
shadowing was resolved prior to beta reduction. The PR
cvc5#11329 refactored this so that beta
redexes for non-rewritten lambdas were lifted to HO terms so that the
rewriter could track proofs without being self referential. However, as
a downside, we now require HO reasoning in first order logics, in
particular, lambdas could be rewritten and beta reduction could be done
for HO applications. The PR cvc5#11513
addresses this shortcoming by allowing non-rewritten lambdas to be
beta-reduced. However, proof elaboration fails since this requires
arbitrarily nested shadow elimination. In particular, a proof of the
above benchmark would (unintuitively) introduce shadowing elimination +
alpha equivalence.

The solution is to apply top-level substitutions + rewriting to *bodies*
of lambdas as they are defined. this prevents HO rewriting while
eliminating complications due to shadowing in beta redexes.

We do not rewrite defined constants, since this complication does not
arise and it better to be lazy based on some benchmarks in SMT-LIB
(cpachecker in QF_UFLRA).

There are several motivations for this change:
1. It makes the proofs significantly simpler and resolves all open
issues related to define-fun (after
cvc5#11513 is merged),
2. It makes the output of e.g. `-o post-asserts` much more intuitive. In
particular, prior to this commit, we'd get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool (@ (lambda ((x Int)) false) x))
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
whereas now we get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool false)
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
@ajreynol ajreynol deleted the lambdaAppElimShadow branch January 21, 2025 20:45
amaleewilson pushed a commit to amaleewilson/cvc5 that referenced this pull request Jan 21, 2025
This ensures that we apply top-level substitutions and rewrite bodies of
define-fun eagerly. This is required for
cvc5#11513.

In particular, currently, due to the fact that variables are *not*
fresh, and the rewriter does not rewrite inside of operators, beta
redexes could appear arbtirarily nested in APPLY_UF. As a result, we
were requiring the fairly sophisticated "shadow elimination" methods for
handling benchmarks such as:
```
(define-fun P ((x Int)) Bool false)
(define-fun Q ((x Int)) Bool (P x))
(assert (Q 0))
(check-sat)
```

In particular, `Q` would be defined to be the lambda `(lambda ((x Int))
(@ (lambda ((x Int)) false) x))`, which assuming the rewriter does not
rewrite inside of operators, leads us to requiring shadow elimination to
do beta reduction.

We previously made a nested call from within the rewriter to ensure such
shadowing was resolved prior to beta reduction. The PR
cvc5#11329 refactored this so that beta
redexes for non-rewritten lambdas were lifted to HO terms so that the
rewriter could track proofs without being self referential. However, as
a downside, we now require HO reasoning in first order logics, in
particular, lambdas could be rewritten and beta reduction could be done
for HO applications. The PR cvc5#11513
addresses this shortcoming by allowing non-rewritten lambdas to be
beta-reduced. However, proof elaboration fails since this requires
arbitrarily nested shadow elimination. In particular, a proof of the
above benchmark would (unintuitively) introduce shadowing elimination +
alpha equivalence.

The solution is to apply top-level substitutions + rewriting to *bodies*
of lambdas as they are defined. this prevents HO rewriting while
eliminating complications due to shadowing in beta redexes.

We do not rewrite defined constants, since this complication does not
arise and it better to be lazy based on some benchmarks in SMT-LIB
(cpachecker in QF_UFLRA).

There are several motivations for this change:
1. It makes the proofs significantly simpler and resolves all open
issues related to define-fun (after
cvc5#11513 is merged),
2. It makes the output of e.g. `-o post-asserts` much more intuitive. In
particular, prior to this commit, we'd get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool (@ (lambda ((x Int)) false) x))
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
whereas now we get:
```
(set-logic ALL)
(define-fun Q ((x Int)) Bool false)
(define-fun P ((x Int)) Bool false)
(assert false)
(check-sat)
```
amaleewilson pushed a commit to amaleewilson/cvc5 that referenced this pull request Jan 21, 2025
…ion (cvc5#11513)

This adds a macro rewrite to capture some of the rewrites in the UF
rewriter and adds proof elaboration for it, which is expected to be
accomplished by (possibly multiple steps of) alpha equivalence.

Note that it uses a more elegant solution than
cvc5#11329, which lifted APPLY_UF to
higher-order applications to ensure shadowing was eliminated. Now, the
rewriting the APPLY_UF is done "in-place", i.e. `(APPLY_UF L ts) --->
(APPLY_UF L' ts)`. As a result, we revert to a behavior where HO_APPLY
is never introduced in first order logics, making the change to TheoryUF
unecessary. It also means we don't rely on higher-order beta-reduction
in first-order logics. This reenables `alethe` on these regressions.

This rewrite is used to ensure that beta reduction does not introduce
capture avoiding substitutions. The UF rewriter is refactored to use the
reference implementation.

Adds a new utility method (getMatchConds) to node_algorith.h which will
be used for a few other pending tasks in proof elaboration.

Fills 1 proof hole in our regressions and 287 proof holes on SMT-LIB.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Priority moderate Complexity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants