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

[Merged by Bors] - refactor: induction₂_symm: Be more explicit in the case variables #11023

Closed
wants to merge 2 commits into from

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Feb 27, 2024

and in setToL1_mono_left, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
induction will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for foralls; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
@nomeata nomeata added awaiting-review The author would like community review of the PR easy < 20s of review time. See the lifecycle page for guidelines. labels Feb 27, 2024
github-merge-queue bot pushed a commit to leanprover/lean4 that referenced this pull request Feb 28, 2024
…rs (#3505)

Else the `case` will now allow introducing all necessary variables.

Induction principles with `let` in the types of the cases will be more
common with #3432.

This implementation no longer reduces the type as it goes, but really
only counts
manifest foralls and lets. I find this more sensible and predictable: If
you have
```
theorem induction₂_symm {P : EReal → EReal → Prop} (symm : Symmetric P) …
```
then previously, writing
```
case symm => 
```
would actually bring a fresh `x` and `y` and variable `h : P x y` into
scope and produce a
goal of `P y x`, because `Symmetric P` happens to be
```
def Symmetric := ∀ ⦃x y⦄, x ≺ y → y ≺ x
```

After this change, after `case symm =>` will leave `Symmetric P` as the
goal.

This gives more control to the author of the induction hypothesis about
the actual
goal of the cases. This shows up in mathlib in two places; fixes in
leanprover-community/mathlib4#11023.
I consider these improvements.
@nomeata
Copy link
Collaborator Author

nomeata commented Feb 28, 2024

The Lean PR has been merged; ideally we merge this into master before it causes extra work for @semorrison and others when they are trying to get nightly-testing to work again.

Copy link
Collaborator

@adomani adomani left a comment

Choose a reason for hiding this comment

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

I do not fully understand the implications of the change, but it looks good to me!

nomeata added a commit that referenced this pull request Feb 29, 2024
and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.

(cherry picked from commit #11023, needed for nightly 2024-02-29)
Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

Thanks 🎉

bors merge

@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review The author would like community review of the PR labels Mar 1, 2024
mathlib-bors bot pushed a commit that referenced this pull request Mar 1, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
@mathlib-bors
Copy link

mathlib-bors bot commented Mar 1, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title refactor: induction₂_symm: Be more explicit in the case variables [Merged by Bors] - refactor: induction₂_symm: Be more explicit in the case variables Mar 1, 2024
@mathlib-bors mathlib-bors bot closed this Mar 1, 2024
@mathlib-bors mathlib-bors bot deleted the joachim/anticipate-lean-pr-3505 branch March 1, 2024 15:31
riccardobrasca pushed a commit that referenced this pull request Mar 1, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
kbuzzard pushed a commit that referenced this pull request Mar 12, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
dagurtomas pushed a commit that referenced this pull request Mar 22, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
utensil pushed a commit that referenced this pull request Mar 26, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
Louddy pushed a commit that referenced this pull request Apr 15, 2024
…1023)

and in `setToL1_mono_left`, instantiate the induction theorem
explicitly.

In leanprover/lean4#3505 I am fixing a big where
`induction` will assume the wrong number of parameters for a case when
there are let-bindings in the type involved. As part of this I am no
longer reducing definitions when looking for `forall`s; this is arguably
more predictable and more likely what the user wants.

There are two breakages in mathlib due to this, and the fix can already
be applied now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy < 20s of review time. See the lifecycle page for guidelines. ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants