Skip to content

chore: adapt to new simp default (decide := false)#7834

Merged
kim-em merged 891 commits intobump/v4.4.0from
lean-pr-testing-2722
Nov 5, 2023
Merged

chore: adapt to new simp default (decide := false)#7834
kim-em merged 891 commits intobump/v4.4.0from
lean-pr-testing-2722

Conversation

@collares
Copy link
Copy Markdown
Contributor

@collares collares commented Oct 22, 2023

Mostly smooth so far, but I think norm_num is less powerful for modulo and divisibility operations on integers. (config := {decide := true}) was added only as a last resort.


There are two test failures left: test/ComputeDegree.lean fails because ComputeDegree is less powerful now because its simp calls (either direct or indirect via norm_num) don't do decidability and can't simplify stuff like 6 ≤ WithBot.unbot' 0 48. Similarly, test/norm_num_ext.lean reveals that norm_num now fails to simplify List.sum and Multiset.sum appropriately. Help is very much appreciated!

Open in Gitpod

@collares collares added the WIP Work in progress label Oct 22, 2023
@collares collares marked this pull request as draft October 22, 2023 11:25
@ghost ghost added blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) labels Oct 22, 2023
@collares collares force-pushed the lean-pr-testing-2722 branch from 8f501cb to b04fb52 Compare October 22, 2023 16:13
@collares collares added awaiting-review help-wanted The author needs attention to resolve issues and removed WIP Work in progress awaiting-review labels Oct 22, 2023
@collares collares force-pushed the lean-pr-testing-2722 branch from 9d835e4 to 828242f Compare October 22, 2023 18:23
@collares collares added awaiting-review and removed help-wanted The author needs attention to resolve issues labels Oct 23, 2023
@collares
Copy link
Copy Markdown
Contributor Author

!bench

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Here are the benchmark results for commit 4d64bfc.
There were significant changes against commit 3ce43c1:

  Benchmark                                                  Metric         Change
  ================================================================================
- ~Mathlib.AlgebraicGeometry.AffineScheme                    instructions     3.2%
+ ~Mathlib.Analysis.Analytic.Basic                           instructions   -17.5%
+ ~Mathlib.Analysis.Analytic.Composition                     instructions   -57.8%
+ ~Mathlib.Analysis.Analytic.Inverse                         instructions   -13.2%
- ~Mathlib.CategoryTheory.Abelian.InjectiveResolution        instructions     6.6%
+ ~Mathlib.CategoryTheory.Abelian.Projective                 instructions    -0.7%
- ~Mathlib.CategoryTheory.Abelian.Transfer                   instructions     4.4%
+ ~Mathlib.CategoryTheory.Preadditive.EilenbergMoore         instructions    -5.3%
- ~Mathlib.Geometry.Euclidean.MongePoint                     instructions     7.6%
- ~Mathlib.LinearAlgebra.FinsuppVectorSpace                  instructions     5.7%
- ~Mathlib.RepresentationTheory.GroupCohomology.Resolution   instructions     3.0%

@collares
Copy link
Copy Markdown
Contributor Author

collares commented Oct 23, 2023

It would have been nice to have some speedups here, but that was not the main point: The real speedups happen when simp tries to decide something and fails after a long time. Those happen in development (see, e.g., https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Regression.20in.20simp) but don't affect mathlib compilation speed.

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Oct 23, 2023

0.4% reduction in instruction count is nothing to sneeze at!

@ghost ghost removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Oct 23, 2023
@collares collares force-pushed the lean-pr-testing-2722 branch from aa2e0a4 to 950cee1 Compare November 3, 2023 10:40
@kim-em kim-em changed the base branch from master to bump/v4.4.0 November 3, 2023 11:16
@collares collares marked this pull request as ready for review November 3, 2023 14:25
@ghost ghost removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Nov 4, 2023
@ghost
Copy link
Copy Markdown

ghost commented Nov 4, 2023

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 4, 2023

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Nov 4, 2023
bors bot pushed a commit that referenced this pull request Nov 4, 2023
Mostly smooth so far, but I think `norm_num` is less powerful for modulo and divisibility operations on integers. `(config := {decide := true})`  was added only as a last resort.



Co-authored-by: Mauricio Collares <mauricio@collares.org>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Nov 4, 2023

Build failed:

@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Nov 4, 2023

bors merge

bors bot pushed a commit that referenced this pull request Nov 4, 2023
Mostly smooth so far, but I think `norm_num` is less powerful for modulo and divisibility operations on integers. `(config := {decide := true})`  was added only as a last resort.



Co-authored-by: Mauricio Collares <mauricio@collares.org>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Nov 4, 2023

Build failed:

@kim-em kim-em merged commit 563cd95 into bump/v4.4.0 Nov 5, 2023
@YaelDillies YaelDillies deleted the lean-pr-testing-2722 branch November 12, 2023 10:07
mathlib-bors bot pushed a commit that referenced this pull request Nov 17, 2023
# PR contents

This is the supremum of

- #8284
- #8056
- #8023
- #8332
- #8226 (already approved)
- #7834 (already approved)

along with some minor fixes from failures on nightly-testing as Mathlib `master` is merged into it.

Note that some PRs for changes that are already compatible with the current toolchain and will be necessary have already been split out: #8380.

I am hopeful that in future we will be able to progressively merge adaptation PRs into a `bump/v4.X.0` branch, so we never end up with a "big merge" like this. However one of these adaptation PRs (#8056) predates my new scheme for combined CI, and it wasn't possible to keep that PR viable in the meantime.

# Lean PRs involved in this bump

In particular this includes adjustments for the Lean PRs

* leanprover/lean4#2778 
* leanprover/lean4#2790
* leanprover/lean4#2783
* leanprover/lean4#2825
* leanprover/lean4#2722

## leanprover/lean4#2778

We can get rid of all the 
```
local macro_rules | `($x ^ $y) => `(HPow.hPow $x $y) -- Porting note: See issue lean4#2220
```
macros across Mathlib (and in any projects that want to write natural number powers of reals).

## leanprover/lean4#2722

Changes the default behaviour of `simp` to `(config := {decide := false})`. This makes `simp` (and consequentially `norm_num`) less powerful, but also more consistent, and less likely to blow up in long failures. This requires a variety of changes: changing some previously by `simp` or `norm_num` to `decide` or `rfl`, or adding `(config := {decide := true})`. 

## leanprover/lean4#2783

This changed the behaviour of `simp` so that `simp [f]` will only unfold "fully applied" occurrences of `f`. The old behaviour can be recovered with `simp (config := { unfoldPartialApp := true })`. We may in future add a syntax for this, e.g. `simp [!f]`; please provide feedback! In the meantime, we have made the following changes:
* switching to using explicit lemmas that have the intended level of application
* `(config := { unfoldPartialApp := true })` in some places, to recover the old behaviour
* Using `@[eqns]` to manually adjust the equation lemmas for a particular definition, recovering the old behaviour just for that definition. See #8371, where we do this for `Function.comp` and `Function.flip`.

This change in Lean may require further changes down the line (e.g. adding the `!f` syntax, and/or upstreaming the special treatment for `Function.comp` and `Function.flip`, and/or removing this special treatment). Please keep an open and skeptical mind about these changes!



Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Mauricio Collares <mauricio@collares.org>
mathlib-bors bot pushed a commit that referenced this pull request Nov 17, 2023
# PR contents

This is the supremum of

- #8284
- #8056
- #8023
- #8332
- #8226 (already approved)
- #7834 (already approved)

along with some minor fixes from failures on nightly-testing as Mathlib `master` is merged into it.

Note that some PRs for changes that are already compatible with the current toolchain and will be necessary have already been split out: #8380.

I am hopeful that in future we will be able to progressively merge adaptation PRs into a `bump/v4.X.0` branch, so we never end up with a "big merge" like this. However one of these adaptation PRs (#8056) predates my new scheme for combined CI, and it wasn't possible to keep that PR viable in the meantime.

# Lean PRs involved in this bump

In particular this includes adjustments for the Lean PRs

* leanprover/lean4#2778 
* leanprover/lean4#2790
* leanprover/lean4#2783
* leanprover/lean4#2825
* leanprover/lean4#2722

## leanprover/lean4#2778

We can get rid of all the 
```
local macro_rules | `($x ^ $y) => `(HPow.hPow $x $y) -- Porting note: See issue lean4#2220
```
macros across Mathlib (and in any projects that want to write natural number powers of reals).

## leanprover/lean4#2722

Changes the default behaviour of `simp` to `(config := {decide := false})`. This makes `simp` (and consequentially `norm_num`) less powerful, but also more consistent, and less likely to blow up in long failures. This requires a variety of changes: changing some previously by `simp` or `norm_num` to `decide` or `rfl`, or adding `(config := {decide := true})`. 

## leanprover/lean4#2783

This changed the behaviour of `simp` so that `simp [f]` will only unfold "fully applied" occurrences of `f`. The old behaviour can be recovered with `simp (config := { unfoldPartialApp := true })`. We may in future add a syntax for this, e.g. `simp [!f]`; please provide feedback! In the meantime, we have made the following changes:
* switching to using explicit lemmas that have the intended level of application
* `(config := { unfoldPartialApp := true })` in some places, to recover the old behaviour
* Using `@[eqns]` to manually adjust the equation lemmas for a particular definition, recovering the old behaviour just for that definition. See #8371, where we do this for `Function.comp` and `Function.flip`.

This change in Lean may require further changes down the line (e.g. adding the `!f` syntax, and/or upstreaming the special treatment for `Function.comp` and `Function.flip`, and/or removing this special treatment). Please keep an open and skeptical mind about these changes!



Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Mauricio Collares <mauricio@collares.org>
alexkeizer pushed a commit that referenced this pull request Nov 17, 2023
# PR contents

This is the supremum of

- #8284
- #8056
- #8023
- #8332
- #8226 (already approved)
- #7834 (already approved)

along with some minor fixes from failures on nightly-testing as Mathlib `master` is merged into it.

Note that some PRs for changes that are already compatible with the current toolchain and will be necessary have already been split out: #8380.

I am hopeful that in future we will be able to progressively merge adaptation PRs into a `bump/v4.X.0` branch, so we never end up with a "big merge" like this. However one of these adaptation PRs (#8056) predates my new scheme for combined CI, and it wasn't possible to keep that PR viable in the meantime.

# Lean PRs involved in this bump

In particular this includes adjustments for the Lean PRs

* leanprover/lean4#2778 
* leanprover/lean4#2790
* leanprover/lean4#2783
* leanprover/lean4#2825
* leanprover/lean4#2722

## leanprover/lean4#2778

We can get rid of all the 
```
local macro_rules | `($x ^ $y) => `(HPow.hPow $x $y) -- Porting note: See issue lean4#2220
```
macros across Mathlib (and in any projects that want to write natural number powers of reals).

## leanprover/lean4#2722

Changes the default behaviour of `simp` to `(config := {decide := false})`. This makes `simp` (and consequentially `norm_num`) less powerful, but also more consistent, and less likely to blow up in long failures. This requires a variety of changes: changing some previously by `simp` or `norm_num` to `decide` or `rfl`, or adding `(config := {decide := true})`. 

## leanprover/lean4#2783

This changed the behaviour of `simp` so that `simp [f]` will only unfold "fully applied" occurrences of `f`. The old behaviour can be recovered with `simp (config := { unfoldPartialApp := true })`. We may in future add a syntax for this, e.g. `simp [!f]`; please provide feedback! In the meantime, we have made the following changes:
* switching to using explicit lemmas that have the intended level of application
* `(config := { unfoldPartialApp := true })` in some places, to recover the old behaviour
* Using `@[eqns]` to manually adjust the equation lemmas for a particular definition, recovering the old behaviour just for that definition. See #8371, where we do this for `Function.comp` and `Function.flip`.

This change in Lean may require further changes down the line (e.g. adding the `!f` syntax, and/or upstreaming the special treatment for `Function.comp` and `Function.flip`, and/or removing this special treatment). Please keep an open and skeptical mind about these changes!



Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Mauricio Collares <mauricio@collares.org>
alexkeizer pushed a commit that referenced this pull request Nov 21, 2023
This is the supremum of

- #8284
- #8056
- #8023
- #8332
- #8226 (already approved)
- #7834 (already approved)

along with some minor fixes from failures on nightly-testing as Mathlib `master` is merged into it.

Note that some PRs for changes that are already compatible with the current toolchain and will be necessary have already been split out: #8380.

I am hopeful that in future we will be able to progressively merge adaptation PRs into a `bump/v4.X.0` branch, so we never end up with a "big merge" like this. However one of these adaptation PRs (#8056) predates my new scheme for combined CI, and it wasn't possible to keep that PR viable in the meantime.

In particular this includes adjustments for the Lean PRs

* leanprover/lean4#2778
* leanprover/lean4#2790
* leanprover/lean4#2783
* leanprover/lean4#2825
* leanprover/lean4#2722

We can get rid of all the
```
local macro_rules | `($x ^ $y) => `(HPow.hPow $x $y) -- Porting note: See issue lean4#2220
```
macros across Mathlib (and in any projects that want to write natural number powers of reals).

Changes the default behaviour of `simp` to `(config := {decide := false})`. This makes `simp` (and consequentially `norm_num`) less powerful, but also more consistent, and less likely to blow up in long failures. This requires a variety of changes: changing some previously by `simp` or `norm_num` to `decide` or `rfl`, or adding `(config := {decide := true})`.

This changed the behaviour of `simp` so that `simp [f]` will only unfold "fully applied" occurrences of `f`. The old behaviour can be recovered with `simp (config := { unfoldPartialApp := true })`. We may in future add a syntax for this, e.g. `simp [!f]`; please provide feedback! In the meantime, we have made the following changes:
* switching to using explicit lemmas that have the intended level of application
* `(config := { unfoldPartialApp := true })` in some places, to recover the old behaviour
* Using `@[eqns]` to manually adjust the equation lemmas for a particular definition, recovering the old behaviour just for that definition. See #8371, where we do this for `Function.comp` and `Function.flip`.

This change in Lean may require further changes down the line (e.g. adding the `!f` syntax, and/or upstreaming the special treatment for `Function.comp` and `Function.flip`, and/or removing this special treatment). Please keep an open and skeptical mind about these changes!

Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Mauricio Collares <mauricio@collares.org>
grunweg pushed a commit that referenced this pull request Dec 15, 2023
# PR contents

This is the supremum of

- #8284
- #8056
- #8023
- #8332
- #8226 (already approved)
- #7834 (already approved)

along with some minor fixes from failures on nightly-testing as Mathlib `master` is merged into it.

Note that some PRs for changes that are already compatible with the current toolchain and will be necessary have already been split out: #8380.

I am hopeful that in future we will be able to progressively merge adaptation PRs into a `bump/v4.X.0` branch, so we never end up with a "big merge" like this. However one of these adaptation PRs (#8056) predates my new scheme for combined CI, and it wasn't possible to keep that PR viable in the meantime.

# Lean PRs involved in this bump

In particular this includes adjustments for the Lean PRs

* leanprover/lean4#2778 
* leanprover/lean4#2790
* leanprover/lean4#2783
* leanprover/lean4#2825
* leanprover/lean4#2722

## leanprover/lean4#2778

We can get rid of all the 
```
local macro_rules | `($x ^ $y) => `(HPow.hPow $x $y) -- Porting note: See issue lean4#2220
```
macros across Mathlib (and in any projects that want to write natural number powers of reals).

## leanprover/lean4#2722

Changes the default behaviour of `simp` to `(config := {decide := false})`. This makes `simp` (and consequentially `norm_num`) less powerful, but also more consistent, and less likely to blow up in long failures. This requires a variety of changes: changing some previously by `simp` or `norm_num` to `decide` or `rfl`, or adding `(config := {decide := true})`. 

## leanprover/lean4#2783

This changed the behaviour of `simp` so that `simp [f]` will only unfold "fully applied" occurrences of `f`. The old behaviour can be recovered with `simp (config := { unfoldPartialApp := true })`. We may in future add a syntax for this, e.g. `simp [!f]`; please provide feedback! In the meantime, we have made the following changes:
* switching to using explicit lemmas that have the intended level of application
* `(config := { unfoldPartialApp := true })` in some places, to recover the old behaviour
* Using `@[eqns]` to manually adjust the equation lemmas for a particular definition, recovering the old behaviour just for that definition. See #8371, where we do this for `Function.comp` and `Function.flip`.

This change in Lean may require further changes down the line (e.g. adding the `!f` syntax, and/or upstreaming the special treatment for `Function.comp` and `Function.flip`, and/or removing this special treatment). Please keep an open and skeptical mind about these changes!



Co-authored-by: leanprover-community-mathlib4-bot <leanprover-community-mathlib4-bot@users.noreply.github.com>
Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Co-authored-by: Mauricio Collares <mauricio@collares.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants