Skip to content

refactor: simplify SimpM recursive definitions#3120

Closed
leodemoura wants to merge 11 commits intomasterfrom
refactor_SimpM
Closed

refactor: simplify SimpM recursive definitions#3120
leodemoura wants to merge 11 commits intomasterfrom
refactor_SimpM

Conversation

@leodemoura
Copy link
Copy Markdown
Member

Depends on #3118.
It breaks Std, but it is an easy fix.

@leodemoura leodemoura requested a review from Kha as a code owner December 27, 2023 22:58
@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 Dec 27, 2023
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Dec 27, 2023
@ghost ghost added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Dec 27, 2023
@ghost
Copy link
Copy Markdown

ghost commented Dec 27, 2023

ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Dec 27, 2023
@leodemoura leodemoura mentioned this pull request Dec 28, 2023
loop : Nat → Expr → Array Expr → Array Expr
| 0, _, as => as
| i+1, .app f a, as => loop i f (as.set! i a)
| _, _, as => as
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will return an array with dummy entries if there aren't enough arguments, will it? Maybe worth pointing out in the docs, also on which side the dummies are? Or maybe just panic instead, its probably misuse if n is too small?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed.

match n, e with
| 0, _ => e
| n+1, .app f _ => extractNumArgs f n
| _, _ => e
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused by the naming; this doesn't extract the args, but extracts the function, or strips the args, doesn't it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed.

The new test exposes a performance problem found in software
verification applications.
See new test for example that takes exponential time without new simp
theorems.
TODO: replace auxiliary theorems with simprocs as soon as we implement them.
The example was looping with the new `simp` reduction strategy. Here
is the looping trace.
```
List.reverseAux (List.reverseAux as []) bs
==> rewrite using reverseAux_reverseAux
List.reverseAux [] (List.reverseAux (List.reverseAux as []) bs)
==> unfold reverseAux
List.reverseAux (List.reverseAux as []) bs
==> rewrite using reverseAux_reverseAux
List.reverseAux [] (List.reverseAux (List.reverseAux as []) bs)
==> ...
```
Motivations:
- We can simplify the big mutual recursion and the implementation.
- We can implement the support for `match`-expressions in the `pre` method.
- It is easier to define and simplify `Simprocs`.
ghost pushed a commit to leanprover-community/mathlib4 that referenced this pull request Dec 30, 2023
Comment on lines +87 to +90
@[simp ↓] theorem ite_cond_true {_ : Decidable c} (a b : α) (h : c) : (if c then a else b) = a := by simp [h]
@[simp ↓] theorem ite_cond_false {_ : Decidable c} (a b : α) (h : ¬ c) : (if c then a else b) = b := by simp [h]
@[simp ↓] theorem dite_cond_true {α : Sort u} {_ : Decidable c} {t : c → α} {e : ¬ c → α} (h : c) : (dite c t e) = t h := by simp [h]
@[simp ↓] theorem dite_cond_false {α : Sort u} {_ : Decidable c} {t : c → α} {e : ¬ c → α} (h : ¬ c) : (dite c t e) = e h := by simp [h]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the intended difference between these lemmas and if_pos/if_neg/dif_pos/dif_neg?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See comment at 5b46fde

@leodemoura
Copy link
Copy Markdown
Member Author

Subsumed by #3124

@leodemoura leodemoura closed this Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan 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