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

feat: decide! #5631

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: decide! #5631

wants to merge 2 commits into from

Conversation

nomeata
Copy link
Contributor

@nomeata nomeata commented Oct 7, 2024

This pull request introduces a new variant of the decide tactic called decide! to speed up the processing of large proofs by skipping proposition checks at the time of elaboration.

This is particularly useful for auto-generated lean proofs.

Fixes #5629

@nomeata nomeata requested a review from Kha as a code owner October 7, 2024 13:23
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc October 7, 2024 13:31 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 Oct 7, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 7, 2024

Mathlib CI status (docs):

  • ❗ Batteries CI can not be attempted yet, as the nightly-testing-2024-10-07 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Batteries CI should run now. (2024-10-07 13:46:26)
  • ✅ Mathlib branch lean-pr-testing-5631 has successfully built against this PR. (2024-10-07 23:06:29) View Log

@tobiasgrosser
Copy link
Contributor

Interesting. We just run into this issue in https://github.com/opencompl/lean-mlir/pull/673/files. In particular, we use native_decide which fails and then run in a kernel error. For some reason try native_decide does not fail, but succeeds and we get an error only on the function overall. This PR is not fixing this, but maybe adds a data-point that there are also automated uses of native_decide that fail and where we trigger the same kernel errors.

@bollu
Copy link
Contributor

bollu commented Oct 7, 2024

The same issue crops up at #5574, where checking the proof to throw an error message when bv_check fails to check the LRAT proof effectively doubles the execution time. So we decided to not go ahead with the approach, but it's a shame, since the error message that results from using an older LRAT proof with a newer bv_check is a little inscrutable.

In an ideal world, there would be some sort of API that can request the kernel to check a proof from Meta code, and returns either a "checked proof handle", or an error, which can then be handled at meta-time.

It seems complex to implement such a feature, though, as the kernel would then need to maintain such "checked proof handles", which is bad for TCB.

@alexkeizer
Copy link
Contributor

It seems complex to implement such a feature, though, as the kernel would then need to maintain such "checked proof handles", which is bad for TCB.

This function already mostly exists, it's called a theorem!

IIRC, a tactic can add new theorems to the environment, so we can have decide or bv_decide generate the proof term. Then, it adds that proof to the environment as a theorem, to get the kernel to check it. If type-checking fails (I assume we can catch that?) we can throw a meaningfull errro in the tactic. Otherwise, if type-checking succeeds, we can emit the new theorem in our original proof, so that we don't have to recheck the proof

@nomeata
Copy link
Contributor Author

nomeata commented Oct 7, 2024

That's clever! This would allow us to half the processing time of decide everywhere - at the expense of lots of theorems proving 0 < 1 over and over again.

Maybe this can hook into the ”abstract proofs” machinery used elsewhere.

@alexkeizer
Copy link
Contributor

We could think of some threshold heuristic, I can imagine that for small statements like 0 < 1 the overhead of adding a new theorem to the kernel could be more than just running the meta checker like we do currently.

Maybe this can hook into the ”abstract proofs” machinery used elsewhere.

I'm not sure I know what you're referring to here, what is this abstract proofs machinery?

@nomeata
Copy link
Contributor Author

nomeata commented Oct 7, 2024

We could think of some threshold heuristic, I can imagine that for small statements like 0 < 1 the overhead of adding a new theorem to the kernel could be more than just running the meta checker like we do currently.

That's a possibility, if we can make it robust (run with a low number of heartbeats). But the incidental complexity and unpredictabiliy of such a design is a bit worrysome.

Maybe this can hook into the ”abstract proofs” machinery used elsewhere.

I'm not sure I know what you're referring to here, what is this abstract proofs machinery?

I'll have to look for it myself, but I've seen .proof_1 definitions show up. Likely something related to

-- `e` is a proof itself. So, we don't abstract nested proofs

@nomeata
Copy link
Contributor Author

nomeata commented Oct 7, 2024

Ah, but it's not the abstracted proofs where I saw lean use previously defined helpers from unrelated definitions, but the match compiler:

def mkMatcherAuxDefinition (name : Name) (type : Expr) (value : Expr) : MetaM (Expr × Option (MatcherInfo → MetaM Unit)) := do

src/Lean/Parser/Tactic.lean Outdated Show resolved Hide resolved
Co-authored-by: Violeta Hernández <[email protected]>
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc October 7, 2024 21:50 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Oct 7, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 7, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Oct 7, 2024
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.

RFC: decide! tactic variant
6 participants