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: omega: more helpful error messages #3847

Merged
merged 8 commits into from
Apr 16, 2024
Merged

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Apr 8, 2024

while trying to help a user who was facing an unhelpful

omega did not find a contradiction:
[0, 0, 0, 0, 1, -1] ∈ [1, ∞)
[0, 0, 0, 0, 0, 1] ∈ [0, ∞)
[0, 0, 0, 0, 1] ∈ [0, ∞)
[1, -1] ∈ [1, ∞)
[0, 0, 0, 1] ∈ [0, ∞)
[0, 1] ∈ [0, ∞)
[1] ∈ [0, ∞)
[0, 0, 0, 1, 1] ∈ [-1, ∞)

I couldn’t resist and wrote a pretty-printer for these problem that
shows the linear combination as such, and includes the recognized atoms.
This is especially useful since oftem omega failures stem from failure
to recognize atoms as equal. In this case, we now get:

omega-failure.lean:19:2-19:7: error: omega could not prove the goal:
a possible counterexample may satisfy the constraints
  d - e ≥ 1
  e ≥ 0
  d ≥ 0
  a - b ≥ 1
  c ≥ 0
  b ≥ 0
  a ≥ 0
  c + d ≥ -1
where
 a := ↑(sizeOf xs)
 b := ↑(sizeOf x)
 c := ↑(sizeOf x.fst)
 d := ↑(sizeOf x.snd)
 e := ↑(sizeOf xs)

and this might help the user make progress (e.g. by using case x
first, and investingating why sizeOf xs shows up twice)

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 8, 2024 22:53 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 Apr 8, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 8, 2024

Mathlib CI status (docs):

  • ❗ Mathlib CI can not be attempted yet, as the nightly-testing-2024-04-08 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-mathlib, Mathlib CI should run now. (2024-04-08 23:01:57)
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-09 19:54:15) View Log
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-09 20:36:33) View Log
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-09 21:08:30) View Log
  • 🟡 Mathlib branch lean-pr-testing-3847 build against this PR was cancelled. (2024-04-10 21:51:33) View Log
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-10 23:07:09) View Log
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-12 07:19:44) View Log
  • ✅ Mathlib branch lean-pr-testing-3847 has successfully built against this PR. (2024-04-13 13:11:24) View Log
  • ❗ Std/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 535427ada498461aa329d941c53bd2a4729513be --onto 1c20b5341956fb77fcf2c2601e64075e51d5f858. (2024-04-16 15:14:01)

@nomeata
Copy link
Collaborator Author

nomeata commented Apr 8, 2024

Draft status to indicate that I don't consider the code finished, but would like to hear if I should go ahead with this, and what changes, if any, you’d like to see for the output.

(I wonder if such non-trivial error messages should be constructed lazily, using ofPPFormat somehow, so that try omega does not get slower.)

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 9, 2024 06:21 Inactive
@nomeata nomeata marked this pull request as ready for review April 9, 2024 06:31
@nomeata nomeata requested a review from kim-em as a code owner April 9, 2024 06:31
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 9, 2024 06:39 Inactive
nomeata added 3 commits April 9, 2024 18:06
while trying to help a user who was facing an unhelpful
```
omega did not find a contradiction:
[0, 0, 0, 0, 1, -1] ∈ [1, ∞)
[0, 0, 0, 0, 0, 1] ∈ [0, ∞)
[0, 0, 0, 0, 1] ∈ [0, ∞)
[1, -1] ∈ [1, ∞)
[0, 0, 0, 1] ∈ [0, ∞)
[0, 1] ∈ [0, ∞)
[1] ∈ [0, ∞)
[0, 0, 0, 1, 1] ∈ [-1, ∞)
```
I couldn’t resist and wrote a pretty-printer for these problem that
shows the linear combination as such, and includes the recognized atoms.
This is especially useful since oftem `omega` failures stem from failure
to recognize atoms as equal. In this case, we now get:

```
omega-failure.lean:17:2-17:7: error: omega could not prove the goal:
a possible counterexample may satisfy the constraints
x₅ - x₆ ∈ [1, ∞)
x₆ ∈ [0, ∞)
x₅ ∈ [0, ∞)
x₁ - x₂ ∈ [1, ∞)
x₄ ∈ [0, ∞)
x₂ ∈ [0, ∞)
x₁ ∈ [0, ∞)
x₄ + x₅ ∈ [-1, ∞)
where
x₁ := ↑(sizeOf xs)
x₂ := ↑(sizeOf x)
x₄ := ↑(sizeOf x.fst)
x₅ := ↑(sizeOf x.snd)
x₆ := ↑(sizeOf xs)
```
and this might help the user make progress (e.g. by using `case x`
first, and investingating why `sizeOf xs` shows up twice)
@nomeata nomeata force-pushed the joachim/omega-error branch from a8cc51a to fee523c Compare April 9, 2024 16:06
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 9, 2024 16:14 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 9, 2024
nomeata added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 9, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Apr 9, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 9, 2024 19:56 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 9, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Apr 9, 2024

I was expecting this to be somewhat expensive in the presence of try omega, but the speedcenter thinks it’s fine:

http://speed.lean-fro.org/mathlib4/compare/a93fa055-e732-498b-8c1f-fd5ab4f48b05/to/fe837d0c-f16b-4edb-981d-f63ef9508ed5

So I guess we can merge this as it is (and maybe use lazy error message construction here once we have it.)

@nomeata nomeata added the awaiting-review Waiting for someone to review the PR label Apr 9, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 9, 2024 20:28 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 9, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 10, 2024 21:17 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 10, 2024
@nomeata nomeata added will-merge-soon …unless someone speaks up and removed awaiting-review Waiting for someone to review the PR labels Apr 10, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 10, 2024 21:36 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 10, 2024
@david-christiansen
Copy link
Contributor

This is really good stuff :-)

But are subscripted xs the easiest to read? Using different letters, perhaps resorting to subscripts when exhausting a name supply, would be easier to distinguish at a glace, I think. Eg:

omega-failure.lean:19:2-19:7: error: omega could not prove the goal:
a possible counterexample may satisfy the constraints
  d - e ≥ 1
  e ≥ 0
  d ≥ 0
  a - b ≥ 1
  c ≥ 0
  b ≥ 0
  a ≥ 0
  c + d ≥ -1
where
  a := ↑(sizeOf xs)
  b := ↑(sizeOf x)
  c := ↑(sizeOf x.fst)
  d := ↑(sizeOf x.snd)
  e := ↑(sizeOf xs)

Otherwise we may end up squinting, especially when working on laptops.

This example is not perfect - It's weird using beginning-of-alphabet for variables rather than constants, and the naming could use late-alphabet letters. But I do think separate names is good.

In either case, I would hope that the name selection avoids things already in scope, which is needed with subscripted variables anyway, as users might do that. And I noticed that there was no x_3 atom in the original message - is that because the atom was unconstrained?

@nomeata nomeata added awaiting-author Waiting for PR author to address issues and removed will-merge-soon …unless someone speaks up labels Apr 12, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Apr 12, 2024

As much as I 🤎 unicode, I fear you are right.

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 12, 2024 06:51 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 12, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 12, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Apr 13, 2024

@david-christiansen, I implemented your idea, I think. What do you think of that?

@nomeata nomeata added awaiting-review Waiting for someone to review the PR and removed awaiting-author Waiting for PR author to address issues labels Apr 13, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 13, 2024 11:53 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 13, 2024
@nomeata nomeata removed the awaiting-review Waiting for someone to review the PR label Apr 16, 2024
@nomeata nomeata added this pull request to the merge queue Apr 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 16, 2024
@nomeata nomeata enabled auto-merge April 16, 2024 14:56
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 16, 2024 15:11 Inactive
@nomeata nomeata added this pull request to the merge queue Apr 16, 2024
Merged via the queue into master with commit 7849724 Apr 16, 2024
11 checks passed
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.

3 participants