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

fix: improve proof search in IndPredBelow #4840

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Jul 26, 2024

MVarId.apply is clever in the order it returns mvars, by putting those
arising from non-dependent arguments first. If we lean on that behavior,
this fixes #1672.

H'T to @DanielFabian, this was found together in a call.

Also improves tracing.

nomeata added 2 commits July 26, 2024 11:32
`MVarId.apply` is clever in the order it returns mvars, by putting those
arising from non-dependent arguments first. If we lean on that behavior,
this fixes #1672.
@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 Jul 26, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Jul 26, 2024

Mathlib CI status (docs):

  • ❗ Batteries CI can not be attempted yet, as the nightly-testing-2024-07-26 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-07-26 09:52:04)
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase f830fc9f4dae8b4d4719b9e886b41a4e2343ddf5 --onto bb9c9bd99f4e81b6199573351a1372482d76541b. (2024-07-28 19:16:12)

@nomeata
Copy link
Collaborator Author

nomeata commented Jul 26, 2024

Ok, we now get test failures because the .brecOn construction succeeds now (previously it was failing silently), and then the function definitions compilation fails in a way that #4827 is likely to fix. So I’ll wait here for that (or its variant #4839).

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 28, 2024 23:43 Inactive
m.apply ctor
let ms ← m.apply ctor
trace[Meta.IndPredBelow] "new goals {ms}"
pure ms.reverse
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change broke some other tests, until I added the reverse here; now the existing tests succeed. 🤷

Note to my future self:

I wonder if this just means that for any order of mvars we’ll have examples where it fails, or if there is really a reason why this order is good™. I’ll have to come back to this and think about it some more.

Maybe I’ll also investigate if we avoid proof search here and build the necessary terms direct, relying on the structure of the .ibelow construction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

IndPreBelow.mkBelow failure
2 participants