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

incorrect functional induction principle #5347

Closed
3 tasks done
BrunoDutertre opened this issue Sep 14, 2024 · 4 comments · Fixed by #5364
Closed
3 tasks done

incorrect functional induction principle #5347

BrunoDutertre opened this issue Sep 14, 2024 · 4 comments · Fixed by #5364
Labels
bug Something isn't working

Comments

@BrunoDutertre
Copy link

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

On the following example, test.induct is incorrect.

def f (x: Nat): Option Nat :=
  if x > 10 then some 0 else none

def test (x: Nat): Nat :=
  match f x, x with
  | some k, _ => k
  | none, 0 => 0
  | none, n + 1 => test n

#check test.induct

It looks like this:

test.induct (motive : Nat → Prop) (case1 : ∀ (x : Nat), motive x) (case2 : motive 0)
  (case3 : ∀ (n : Nat), motive n → motive n.succ) (x : Nat) : motive x

and case1 is missing an assumption like f x = some k.

Steps to Reproduce

Run lean on the example.

Expected behavior:

Something like this:

test.induct (motive : Nat → Prop) (case1 : ∀ (x k : Nat), f x = some k → motive x) (case2 : motive 0)
  (case3 : ∀ (n : Nat), motive n → motive n.succ) (x : Nat) : motive x

Actual behavior:

Wrong induction principle.

Versions

"4.11.0"

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@BrunoDutertre BrunoDutertre added the bug Something isn't working label Sep 14, 2024
@nomeata
Copy link
Collaborator

nomeata commented Sep 14, 2024

Thanks for the report! I'll investigate. If I'm lucky then #4149 might already fix it, we'll see.

I was hoping that two separate match statements might fix this, but it fails even worse:
https://live.lean-lang.org/#codez=CYUwZgBJAUAeBcEByBDALgSkQeQA5oEsB7AO2XQngF4AoCCAyWCAPggEYAGCNACxDIBnIgFsQEbiAA2g8SVIgadCKEhoQgtBDiJUmXRWrL6I9AGNeES4iYQA7gT4RjEAD4RhYiAGsIVNt4u7vIk4v4QpmgWEMwOfC70iW4SfmycCUnJZADUHKk8GlokSpmZAMQWIGa+6poAdAQkwACuZmhAA

@BrunoDutertre
Copy link
Author

I tried nested match statements but that can cause another problem: #2962

@nomeata
Copy link
Collaborator

nomeata commented Sep 15, 2024

A quick test confirms my worry that the MVarId.cleanup step is at fault:

[Meta.FunInd] Goal before cleanup:
case hyp
motive : Nat → Prop
t : Nat
f : Nat.ibelow t
k x✝² : Nat
x✝¹ : _root_.f t = some k
x✝ : Nat.ibelow x✝²
⊢ motive x✝²
 

[Meta.FunInd] Goal after cleanup (toClear := [f, x✝]) (toPreserve := []):
case hyp
motive : Nat → Prop
x✝ : Nat
⊢ motive x✝

But if I put too much stuff into toPreserve array, then we also get induction principles that are not nice, because they contain completely unused variables, e.g.

info: ackermann.induct (motive : Nat → Nat → Prop) (case1 : Nat → Nat → ∀ (m : Nat), motive 0 m)
  (case2 : Nat → Nat → ∀ (n : Nat), motive n 1 → motive n.succ 0)
  (case3 : Nat → Nat → ∀ (n m : Nat), motive (n + 1) m → motive n (ackermann (n + 1) m) → motive n.succ m.succ) :
  ∀ (a a_1 : Nat), motive a a_1

This may need more careful thought, or a possibly a completely different approach. (Maybe instead of the blunt MVarId.cleanUp, only clean up the context very selectively to not remove useful assumptions.)

nomeata added a commit that referenced this issue Sep 15, 2024
@nomeata
Copy link
Collaborator

nomeata commented Sep 15, 2024

Ah, here is a partial work-around for you: If you put h : x in the match you get the assumption at least in two of the three cases:

def f (x: Nat): Option Nat :=
  if x > 10 then some 0 else none

def test (x: Nat): Nat :=
  match f x, h : x with
  | some k, _ => k
  | none, 0 => 0
  | none, n + 1 => test n

/--
info: test.induct (motive : Nat → Prop) (case1 : ∀ (k x : Nat), f x = some k → motive x) (case2 : motive 0)
  (case3 : ∀ (n : Nat), f n.succ = none → motive n → motive n.succ) (x : Nat) : motive x
-/
#guard_msgs in
#check test.induct

But yes, this is not satisfactory.

github-merge-queue bot pushed a commit that referenced this issue Sep 15, 2024
not a fix, unfortunately, just recording the test.
github-merge-queue bot pushed a commit that referenced this issue Sep 16, 2024
A round of clean-up for the context of the functional induction
principle cases.

* Already previously, with `match e with | p => …`, functional induction
would ensure that `h : e = p` is in scope, but it wouldn’t work in
dependent cases. Now it introduces heterogeneous equality where needed
(fixes #4146)
* These equalities are now added always (previously we omitted them when
the discriminant was a variable that occurred in the goal, on the
grounds that the goal gets refined through the match, but it’s more
consistent to introduce the equality in any case)
* We no longer use `MVarId.cleanup` to clean up the goal; it was
sometimes too aggressive (fixes #5347)
* Instead, we clean up more carefully and with a custom strategy:
* First, we substitute all variables without a user-accessible name, if
we can.
  * Then, we substitute all variable, if we can, outside in.
* As we do that, we look for `HEq`s that we can turn into `Eq`s to
substitute some more
  * We substitute unused `let`s.
  
**Breaking change**: In some cases leads to a different functional
induction principle (different names and order of assumptions, for
example).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants