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

refactor: cleanup simp types and pre and post method semantics #3210

Merged
merged 23 commits into from
Feb 1, 2024

Conversation

leodemoura
Copy link
Member

@leodemoura leodemoura commented Jan 23, 2024

Before this commit, Simprocs were defined as Expr -> SimpM (Option Step), where Step is inductively defined as follows:

inductive Step where
  | visit : Result → Step
  | done  : Result → Step

Here, Result is a structure containing the resulting expression and a proof demonstrating its equality to the input. Notably, the proof is optional; in its absence, simp assumes reflexivity.

A simproc can:

  • Fail by returning none, indicating its inapplicability. In this case, the next suitable simproc is attempted, along with other simp extensions.
  • Succeed and invoke further simplifications using the .visit
    constructor. This action returns control to the beginning of the
    simplification loop.
  • Succeed and indicate that the result should not undergo further
    simplifications. However, I find the current approach unsatisfactory, as it does not align with the methodology employed in Transform.lean, where we have the type:
inductive TransformStep where
  /-- Return expression without visiting any subexpressions. -/
  | done (e : Expr)
  /--
  Visit expression (which should be different from current expression) instead.
  The new expression `e` is passed to `pre` again.
  -/
  | visit (e : Expr)
  /--
  Continue transformation with the given expression (defaults to current expression).
  For `pre`, this means visiting the children of the expression.
  For `post`, this is equivalent to returning `done`. -/
  | continue (e? : Option Expr := none)

This type makes it clearer what is going on. The new Simp.Step type is similar but use Result instead of Expr because we need a proof.

Other modifications:

  • The pre and post methods have now type Simproc.
  • simprocs field at Methods was deleted.
  • Added andThen combinator for Simproc.
  • Cleanup simpLoop semantics.

Todo: cleanup reduceStep.

Recording adapation PRs here:

@leodemoura leodemoura requested review from kim-em and Kha as code owners January 23, 2024 04:27
@leodemoura leodemoura marked this pull request as draft January 23, 2024 04:28
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jan 23, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jan 23, 2024

Mathlib CI status (docs):

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc January 23, 2024 05:14 Inactive
@leodemoura
Copy link
Member Author

The register_simp_attr <id> command now also creates a simproc set.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 24, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc January 24, 2024 02:11 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 30, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jan 30, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan full-ci labels Jan 30, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc January 30, 2024 03:31 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 30, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jan 30, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc January 30, 2024 21:10 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 30, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jan 30, 2024
leodemoura added a commit to leanprover-community/batteries that referenced this pull request Jan 30, 2024
@leodemoura leodemoura marked this pull request as ready for review January 30, 2024 23:03
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jan 30, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jan 30, 2024
kim-em added a commit to leanprover-community/aesop that referenced this pull request Jan 31, 2024
When we declare a `simp` set using `register_simp_attr`, we
automatically create `simproc` set. However, users may create `simp`
sets programmatically, and the associated `simproc` set may be missing
and vice-versa.
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc February 1, 2024 01:27 Inactive
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Feb 1, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Feb 1, 2024
@kim-em kim-em merged commit a4226a4 into master Feb 1, 2024
24 of 32 checks passed
kim-em added a commit to leanprover-community/aesop that referenced this pull request Feb 1, 2024
Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>
Co-authored-by: Leonardo de Moura <leomoura@amazon.com>
Co-authored-by: Jannis Limperg <jannis@limperg.de>
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Feb 2, 2024
This is the adaptation PR for nightly-2024-02-01.

It rolls in the branches
* #9843, prepared by @mattrobball, which has the adaptations for
leanprover/lean4#2478
* #10133, prepared by @semorrison, which has the adaptations for
leanprover/lean4#3210

As these both landed in the same nightly, we're having to do the update
in one go.

Note this nightly is intended to become `v4.6.0-rc1` tomorrow.

---
<!-- The text above the `---` will become the commit message when your
PR is merged. Please leave a blank newline before the `---`, otherwise
GitHub will format the text above it as a title.

To indicate co-authors, include lines at the bottom of the commit
message
(that is, before the `---`) using the following format:

Co-authored-by: Author Name <author@email.com>

Any other comments you want to keep out of the PR commit should go
below the `---`, and placed outside this HTML comment, or else they
will be invisible to reviewers.

If this PR depends on other PRs, please list them below this comment,
using the following format:
- [ ] depends on: #abc [optional extra text]
- [ ] depends on: #xyz [optional extra text]
-->

[![Open in
Gitpod](https://gitpod.io/button/open-in-gitpod.svg)](https://gitpod.io/from-referrer/)

---------

Co-authored-by: Kevin Buzzard <k.buzzard@imperial.ac.uk>
Co-authored-by: Matthew Ballard <matt@mrb.email>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants