-
Notifications
You must be signed in to change notification settings - Fork 717
feat: optimized simp routine for let telescopes #8968
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
Changes from all commits
fe5ab3a
43c57be
70981dd
af87540
e31de09
cce9814
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,22 @@ theorem let_body_congr {α : Sort u} {β : α → Sort v} {b b' : (a : α) → | |
| (a : α) (h : ∀ x, b x = b' x) : (let x := a; b x) = (let x := a; b' x) := | ||
| (funext h : b = b') ▸ rfl | ||
|
|
||
| /-! | ||
| Congruence lemmas for `have` have kernel performance issues when stated using `have` directly. | ||
| Illustration of the problem: the kernel infers that the type of | ||
| `have_congr (fun x => b) (fun x => b') h₁ h₂` | ||
| is | ||
| `(have x := a; (fun x => b) x) = (have x := a'; (fun x => b') x)` | ||
| rather than | ||
| `(have x := a; b x) = (have x := a'; b' x)` | ||
| That means the kernel will do `whnf_core` at every step of checking a sequence of these lemmas. | ||
| Thus, we get quadratically many zeta reductions. | ||
|
|
||
| For reference, we have the `have` versions of the theorems in the following comment, | ||
| and then after that we have the versions that `simpHaveTelescope` actually uses, | ||
| which avoid this issue. | ||
| -/ | ||
| /- | ||
| theorem have_unused {α : Sort u} {β : Sort v} (a : α) {b b' : β} | ||
| (h : b = b') : (have _ := a; b) = b' := h | ||
|
|
||
|
|
@@ -95,6 +111,29 @@ theorem have_body_congr_dep {α : Sort u} {β : α → Sort v} (a : α) {f f' : | |
| theorem have_body_congr {α : Sort u} {β : Sort v} (a : α) {f f' : α → β} | ||
| (h : ∀ x, f x = f' x) : (have x := a; f x) = (have x := a; f' x) := | ||
| h a | ||
| -/ | ||
|
|
||
| theorem have_unused' {α : Sort u} {β : Sort v} (a : α) {b b' : β} | ||
| (h : b = b') : (fun _ => b) a = b' := h | ||
|
|
||
| theorem have_unused_dep' {α : Sort u} {β : Sort v} (a : α) {b : α → β} {b' : β} | ||
| (h : ∀ x, b x = b') : b a = b' := h a | ||
|
|
||
| theorem have_congr' {α : Sort u} {β : Sort v} {a a' : α} {f f' : α → β} | ||
| (h₁ : a = a') (h₂ : ∀ x, f x = f' x) : f a = f' a' := | ||
| @congr α β f f' a a' (funext h₂) h₁ | ||
|
|
||
| theorem have_val_congr' {α : Sort u} {β : Sort v} {a a' : α} {f : α → β} | ||
| (h : a = a') : f a = f a' := | ||
| @congrArg α β a a' f h | ||
|
|
||
| theorem have_body_congr_dep' {α : Sort u} {β : α → Sort v} (a : α) {f f' : (x : α) → β x} | ||
| (h : ∀ x, f x = f' x) : f a = f' a := | ||
| h a | ||
|
|
||
| theorem have_body_congr' {α : Sort u} {β : Sort v} (a : α) {f f' : α → β} | ||
| (h : ∀ x, f x = f' x) : f a = f' a := | ||
| h a | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kmill As far as I understood, the theorems above are a workaround for the performance issue, and are not stated as you wanted to state them. If I understood it correctly, could you please include in the comment the desired version you wanted to have?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The desired ones are in the comment. (I added the desired ones in a previous PR, and in this PR I commented them out to make sure I didn't accidentally use them.) I haven't confirmed it yet, but I think there's a similar performance issue still with these |
||
|
|
||
| theorem letFun_unused {α : Sort u} {β : Sort v} (a : α) {b b' : β} (h : b = b') : @letFun α (fun _ => β) a (fun _ => b) = b' := | ||
| h | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kmill I will add this issue to my todo list for next quarter. Could you please send me examples that expose the quadratic behavior? Have you checked whether the external checker written in Rust also has this performance issue?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The deep telescopes at the ends of
simpHave.leanandsimpLetFunIssue.leanillustrate the issue. They also have a separate issue arising from usingsimpto unfold a recursive function that introduces ahaveto the telescope. (This is what theid idhack is addressing.)I'll make some more examples for you next quarter.
I haven't had a chance to compare this with external checkers yet. I would be surprised if they did not have a similar issue.