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

Timeout with large arithmetic expression involving numerals #4861

Open
3 tasks done
TwoFX opened this issue Jul 29, 2024 · 5 comments
Open
3 tasks done

Timeout with large arithmetic expression involving numerals #4861

TwoFX opened this issue Jul 29, 2024 · 5 comments
Labels
bug Something isn't working P-medium We may work on this issue if we find the time

Comments

@TwoFX
Copy link
Member

TwoFX commented Jul 29, 2024

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

The following code timeouts with the default maxHeartbeats setting. It sometimes fails at whnf and sometimes at isDefEq, depending on the specific heartbeat setting used.

theorem foo (x y z p q : Int) : False :=
  have : (1 * x ^ 1 + z ^ 1 * p) *
        (1 / 1 * p ^ 1 * x ^ 1 + 1 * q * p ^ 1 * x * z + 1 * q ^ 1 * p ^ 1 * x ^ 1 +
                            1 * q ^ 1 * p ^ 1 * x * z -
                          1 * q * p ^ 1 * y ^ 1 +
                        1 * q ^ 1 * p ^ 1 * x ^ 1 +
                      1 * q ^ 1 * p ^ 1 * x * z -
                    1 * q ^ 1 * p ^ 1 * y ^ 1 +
                  1 * q ^ 1 * p ^ 1 * x ^ 1 +
                1 * q ^ 1 * p ^ 1 * x * z -
              1 * q ^ 1 * p ^ 1 * y ^ 1 +
            1 * q ^ 1 * x ^ 1 -
          1 * q ^ 1 * p * y ^ 1) +
      z * (1 * y) *
        (-(1 / 1 * p ^ 1 * x * y) + 1 * q * p ^ 1 * z * y - 1 * q ^ 1 * p ^ 1 * x * y +
                  1 * q ^ 1 * p ^ 1 * z * y -
                1 * q ^ 1 * p ^ 1 * x * y +
              1 * q ^ 1 * p ^ 1 * z * y -
            1 * q ^ 1 * p ^ 1 * x * y +
          1 / 1 * q ^ 1 * p ^ 1 * z * y) +
    (-y ^ 1 + p * x * (1 * z) + q * (1 * z ^ 1)) *
      (-(1 / 1 * p ^ 1 * x * z) - 1 * q * p ^ 1 * x ^ 1 - 1 * q ^ 1 * p ^ 1 * x * z -
                1 * q ^ 1 * p ^ 1 * x ^ 1 -
              1 * q ^ 1 * p ^ 1 * x * z -
            1 * q ^ 1 * p ^ 1 * x ^ 1 -
          1 * q ^ 1 * p ^ 1 * x * z -
        1 * q ^ 1 * p * x ^ 1) =
  1 *
        (1 / 1 * p ^ 1 * x ^ 1 + 1 * q * p ^ 1 * x * z + 1 * q ^ 1 * p ^ 1 * x ^ 1 +
                            1 * q ^ 1 * p ^ 1 * x * z -
                          1 * q * p ^ 1 * y ^ 1 +
                        1 * q ^ 1 * p ^ 1 * x ^ 1 +
                      1 * q ^ 1 * p ^ 1 * x * z -
                    1 * q ^ 1 * p ^ 1 * y ^ 1 +
                  1 * q ^ 1 * p ^ 1 * x ^ 1 +
                1 * q ^ 1 * p ^ 1 * x * z -
              1 * q ^ 1 * p ^ 1 * y ^ 1 +
            1 * q ^ 1 * x ^ 1 -
          1 * q ^ 1 * p * y ^ 1) +
      1 *
        (-(1 / 1 * p ^ 1 * x * y) + 1 * q * p ^ 1 * z * y - 1 * q ^ 1 * p ^ 1 * x * y +
                  1 * q ^ 1 * p ^ 1 * z * y -
                1 * q ^ 1 * p ^ 1 * x * y +
              1 * q ^ 1 * p ^ 1 * z * y -
            1 * q ^ 1 * p ^ 1 * x * y +
          1 / 1 * q ^ 1 * p ^ 1 * z * y) +
    1 *
      (-(1 / 1 * p ^ 1 * x * z) - 1 * q * p ^ 1 * x ^ 1 - 1 * q ^ 1 * p ^ 1 * x * z -
                1 * q ^ 1 * p ^ 1 * x ^ 1 -
              1 * q ^ 1 * p ^ 1 * x * z -
            1 * q ^ 1 * p ^ 1 * x ^ 1 -
          1 * q ^ 1 * p ^ 1 * x * z -
        1 * q ^ 1 * p * x ^ 1) := sorry
  sorry

The problem goes away when introducing a new parameter w : Nat and replacing all 1 by w.

set_option diagnostics true shows some interesting type class numbers:

[type_class] used instances (max: 42539, num: 13): ▼
  USize.instOfNat ↦ 42539
  Int.instHPowNat ↦ 42145
  Lean.Omega.IntList.instHMulInt ↦ 21357
  instHMul ↦ 11896
  instPowNat ↦ 8621
  instHSubPos ↦ 5140
  instHAddPosChar ↦ 4084
  Lean.Omega.IntList.instNeg ↦ 845
  instHAdd ↦ 440
  instHSub ↦ 189
  Int.instMul ↦ 90
  Int.instAdd ↦ 31
  Int.instSub ↦ 29

Context

The linear_combination tactic in mathlib is suffering from performance issues, and handling terms that look like the example above seems to be part of the reason why it is so slow.

Steps to Reproduce

  1. Copy the above code into the Lean web editor.

Expected behavior: Elaboration should be very fast

Actual behavior: Timeouts

Versions

4.11.0-nightly-2024-07-27 on live.lean-lang.org

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@TwoFX TwoFX added the bug Something isn't working label Jul 29, 2024
@kokic
Copy link

kokic commented Jul 30, 2024

Yes, a simple way to avoid timeout is to split the expression into multiple functions or definitions.

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Aug 2, 2024
leodemoura added a commit that referenced this issue Aug 2, 2024
This modification improves the performance of the example in issue #4861.
It no longer times out but is still expensive.

Here is the analysis of the performance issue: Given `(x : Int)`, to elaborate `x ^ 1`, a few default instances have to be tried.

First, the homogeneous instance is tried and fails since `Int` does
not implement `Pow Int`. Then, the `NatPow` instance is tried, and it
also fails. The same process is performed for each term of the form `p
^ 1`. There are seveal of them at #4861. After all of these fail,
the lower priority default instance for numerals is tried, and
`x ^ 1` becomes `x ^ (1 : Nat)`. Then, `HPow Int Nat Int` can be
applied, and the elaboration succeeds. However, this process has
to be repeated for every single term of the form `p ^ 1`.
The elaborator tries all homogeneous `HPow` and `NatPow` instances
for all `p ^ 1` terms before trying the lower priority default instance `OfNat`.

This commit ensures `Int` has a `NatPow` instance instead of `HPow Int
Nat Int`. This change shortcuts the process, but it still first
tries the homogeneous `HPow` instance, fails, and then tries
`NatPow`. The elaboration can be made much more efficient by writing `p ^ (1 : Nat)`.
@leodemoura
Copy link
Member

leodemoura commented Aug 2, 2024

@TwoFX @Kha The PR above has an analysis for this issue. With the PR, the example does not time out anymore, but is still expensive. Simple workaround for much better performance: write x ^ (1 : Nat) instead of x ^ 1. The PR also makes this suggestion.

github-merge-queue bot pushed a commit that referenced this issue Aug 3, 2024
This modification improves the performance of the example in issue
#4861. It no longer times out but is still expensive.

Here is the analysis of the performance issue: Given `(x : Int)`, to
elaborate `x ^ 1`, a few default instances have to be tried.

First, the homogeneous instance is tried and fails since `Int` does not
implement `Pow Int`. Then, the `NatPow` instance is tried, and it also
fails. The same process is performed for each term of the form `p ^ 1`.
There are seveal of them at #4861. After all of these fail, the lower
priority default instance for numerals is tried, and `x ^ 1` becomes `x
^ (1 : Nat)`. Then, `HPow Int Nat Int` can be applied, and the
elaboration succeeds. However, this process has to be repeated for every
single term of the form `p ^ 1`. The elaborator tries all homogeneous
`HPow` and `NatPow` instances for all `p ^ 1` terms before trying the
lower priority default instance `OfNat`.

This commit ensures `Int` has a `NatPow` instance instead of `HPow Int
Nat Int`. This change shortcuts the process, but it still first tries
the homogeneous `HPow` instance, fails, and then tries `NatPow`. The
elaboration can be made much more efficient by writing `p ^ (1 : Nat)`.
@kmill
Copy link
Collaborator

kmill commented Aug 6, 2024

For a performance measurement, the example spends about 3% of the elaboration time inside the binop elaborator, and then I am guessing the rest of the time it is in the synthetic metavariable synthesis loop.

I thought about using withSynthesize at various points in the binop elaborator to eagerly use default instances, but default instances need to wait for additional type information in reasonable cases. For example, in the following you don't know that 2 : Float until after xs is elaborated, using the default instance coming from the HomogeneousPow Float instance. If we attempted default instances earlier, it would commit to 2 : Nat, leading to failure.

variable (xs : Array Float)
#check Array.map (fun x => x ^ 2) xs

One thing we could do to dramatically reduce the number of instance problems in this example is to re-use the elaborated functions when the types of both operands are known in toExprCore, using a cache keyed by operand types. This should be easy to implement and not change elaboration except for performance. I'll investigate.

@hrmacbeth
Copy link
Contributor

Thanks for investigating this! Just a note that in the terms produced by linear_combination, the variables might come from any type equipped with a CommRing instance, not just the integers.

(Not sure which of the fixes being investigated still work in this setting.)

@kmill
Copy link
Collaborator

kmill commented Aug 7, 2024

In Heather's linear_combination examples, slow exponentiation dominates. Over at leanprover-community/mathlib4#15599 I made a version of the tactic that fully elaborates exponents before passing expressions to the elaborator, which made it take 8% the time it used to.

We can't do this in general because we want to support HomogenousPow for Float applications. I think we could measure the performance impact of having binop fully elaborate exponents for mathlib, to see if it's worth thinking about redesigning how NatPow and HomogeneousPow work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P-medium We may work on this issue if we find the time
Projects
None yet
Development

No branches or pull requests

6 participants