Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

fix(tactic/core): remove all_goals option from apply_any#2275

Merged
mergify[bot] merged 3 commits intomasterfrom
solve_by_elim_bug
Mar 29, 2020
Merged

fix(tactic/core): remove all_goals option from apply_any#2275
mergify[bot] merged 3 commits intomasterfrom
solve_by_elim_bug

Conversation

@kim-em
Copy link
Collaborator

@kim-em kim-em commented Mar 29, 2020

Fixes the -T50000 regression just noticed by Kevin.

#2245 introduced a regression, essentially breaking set_theory/pgame.lean. This wasn't caught by CI because it's a performance regression, which made solve_by_elim enormously slower on some tasks.

The problem was that I added an all_goals option to apply_any, which told it to try to apply one of its lemmas to any of the goals.

  1. This is a useless feature, as apply_any lemmas {all_goals := tt} is equivalent to any_goal {apply_any lemmas}.
  2. Worse, I set the default value of all_goals to true. In some situations, this now causes solve_by_elim to try solving unsolvable subgoals in all possible orders, causing a combinatorial slowdown.

This PR simply removes the all_goals option from apply_any. (See point 1!) Note that solve_by_elim*, which tries to solve all goals at once, still works exactly as before, and as expected --- as long as you expect that its behaviour is to first solve the first goal, then try to solve the second goal, backtracking to a different solution of the first goal if that fails, and so on.

The file src/set_theory/pgame.lean now compiles again all the way down to -T6000.

(I also took the opportunity to remove some unnecessary imports from this file, and I promise this is orthogonal to the slowdown issue!)

Loading
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants