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

refactor: make 1 % n reduce without well-founded recursion #4098

Merged
merged 4 commits into from
May 8, 2024

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented May 7, 2024

this is in preparation for #4061. Once that lands, 1 % 42 = 1 will no
longer hold definitionally (at least not without an ungly unseal Nat.modCore in around). This affects mathlib in a few places,
essentially every time a 1 : Fin (n+1) literal is written.

So this extends the existing special case for 0 % n = 0 to 1 % n.

this is in preparation for #4061. Once that lands, `1 % 42 = 1` will no
longer hold definitionally (at least not without an ungly `unseal
Nat.modCore in` around). This affects mathlib in a few places,
essentially every time a `1 : Fin (n+1)` literal is written.

So this extends the existing special case for `0 % n = 0` to `1 % n`.
src/Init/Data/Nat/Div.lean Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 7, 2024 21:28 Inactive
@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 May 7, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 7, 2024

Mathlib CI status (docs):

  • ❗ Std CI can not be attempted yet, as the nightly-testing-2024-05-07 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Std CI should run now. (2024-05-07 21:32:01)
  • ✅ Mathlib branch lean-pr-testing-4098 has successfully built against this PR. (2024-05-08 08:11:16) View Log
  • ✅ Mathlib branch lean-pr-testing-4098 has successfully built against this PR. (2024-05-08 08:52:37) View Log
  • ✅ Mathlib branch lean-pr-testing-4098 has successfully built against this PR. (2024-05-08 10:25:22) View Log

@nomeata nomeata closed this May 8, 2024
@nomeata nomeata reopened this May 8, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 8, 2024 06:56 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label May 8, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 8, 2024 08:13 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@nomeata nomeata marked this pull request as ready for review May 8, 2024 08:40
@nomeata nomeata requested a review from semorrison as a code owner May 8, 2024 08:40
nomeata added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@nomeata nomeata added the awaiting-review Waiting for someone to review the PR label May 8, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 8, 2024 09:35 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 8, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
nomeata added a commit to leanprover-community/mathlib4 that referenced this pull request May 8, 2024
@leodemoura leodemoura removed the awaiting-review Waiting for someone to review the PR label May 8, 2024
@nomeata nomeata added this pull request to the merge queue May 8, 2024
Merged via the queue into master with commit 227e861 May 8, 2024
38 checks passed
@nomeata nomeata deleted the joachim/nat-mod-defeq branch May 8, 2024 15:54
github-merge-queue bot pushed a commit that referenced this pull request May 10, 2024
we keep running into examples where working with well-founded recursion
is slow because defeq checks (which are all over the place, including
failing ones that are back-tracked) unfold well-founded definitions.

The definition of a function defined by well-founded recursion should be
an implementation detail that should only be peeked inside by the
equation generator and the functional induction generator.

We now mark the mutual recursive function as irreducible (if the user
did not
set a flag explicitly), and use `withAtLeastTransparency .all` when
producing
the equations.

Proofs can be fixed by using rewriting, or – a bit blunt, but nice for
adjusting
existing proofs – using `unseal` (a.k.a. `attribute [local
semireducible]`).

Mathlib performance does not change a whole lot:

http://speed.lean-fro.org/mathlib4/compare/08b82265-75db-4a28-b12b-08751b9ad04a/to/16f46d5e-28b1-41c4-a107-a6f6594841f8
Build instructions -0.126 %, four modules with significant instructions
decrease.

To reduce impact, these definitions were changed:

* `Nat.mod`, to make `1 % n` reduce definitionally, so that `1` as a
`Fin 2` literal
works nicely. Theorems with larger `Fin` literals tend to need a `unseal
Nat.modCore`
   #4098
* `List.ofFn` rewritten to be structurally recursive and not go via
`Array.ofFn`:
   leanprover-community/batteries#784

Alternative designs explored were

 * Making `WellFounded.fix` irreducible. 
 
One benefit is that recursive functions with equal definitions (possibly
after
instantiating fixed parameters) are defeq; this is used in mathlib to
relate

[`OrdinalApprox.gfpApprox`](https://leanprover-community.github.io/mathlib4_docs/Mathlib/SetTheory/Ordinal/FixedPointApproximants.html#OrdinalApprox.gfpApprox)
with `.lfpApprox`.
   
   But the downside is that one cannot use `unseal` in a
targeted way, being explicit in which recursive function needs to be
reducible here.

And in cases where Lean does unwanted unfolding, we’d still unfold the
recursive
definition once to expose `WellFounded.fix`, leading to large terms for
often no good
   reason.

* Defining `WellFounded.fix` to unroll defintionally once before hitting
a irreducible
`WellFounded.fixF`. This was explored in #4002. It shares most of the
ups and downs
with the previous variant, with the additional neat benefit that
function calls that
do not lead to recursive cases (e.g. a `[]` base case) reduce nicely.
This means that
   the majority of existing `rfl` proofs continue to work.

Issue #4051, which demonstrates how badly things can go if wf recursive
functions can be
unrolled, showed that making the recursive function irreducible there
leads to noticeably
faster elaboration than making `WellFounded.fix` irreducible; this is
good evidence that
the present PR is the way to go. 

This fixes #3988

---------

Co-authored-by: Leonardo de Moura <[email protected]>
nomeata added a commit that referenced this pull request May 13, 2024
this refined upon #4098 and makes `Nat.mod` reduce on even more
literals. The key observation that I missed earlier is that `if m ≤ n`
reduces better than `if n < m`.

Also see discussion at
leanprover-community/mathlib4#12853 (comment)
github-merge-queue bot pushed a commit that referenced this pull request May 13, 2024
this refined upon #4098 and makes `Nat.mod` reduce on even more
literals. The key observation that I missed earlier is that `if m ≤ n`
reduces better than `if n < m`.

Also see discussion at

leanprover-community/mathlib4#12853 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR 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.

None yet

3 participants