Skip to content

[Merged by Bors] - chore: simplify how solve_by_elim works when not backtracking#3480

Closed
kim-em wants to merge 9 commits intomasterfrom
simplify_backtracking_solve_by_elim
Closed

[Merged by Bors] - chore: simplify how solve_by_elim works when not backtracking#3480
kim-em wants to merge 9 commits intomasterfrom
simplify_backtracking_solve_by_elim

Conversation

@kim-em
Copy link
Copy Markdown
Contributor

@kim-em kim-em commented Apr 17, 2023

We had some unfortunate spaghetti code in solve_by_elim. When @hrmacbeth had requested additional features for apply_rules, the easiest way to provide them was to re-use solve_by_elim's parsing and lemma handling. (See #856.) However apply_rules doesn't to backtracking, and solve_by_elim is all about it. At the time, solve_by_elim didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in apply_rules). This PR does that.


Open in Gitpod

@kim-em kim-em added awaiting-review awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) labels Apr 17, 2023
@kim-em kim-em removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Apr 17, 2023
@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Apr 17, 2023

This PR/issue depends on:

@github-actions github-actions bot removed the awaiting-CI This PR does not pass CI yet. This label is automatically removed once it does. label Apr 17, 2023
@kim-em kim-em requested a review from hrmacbeth April 18, 2023 03:31
@kim-em kim-em added the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 19, 2023
@gebner gebner added the t-meta Tactics, attributes or user commands label Apr 19, 2023
@kim-em kim-em removed the merge-conflict The PR has a merge conflict with master, and needs manual merging. (this label is managed by a bot) label Apr 20, 2023
@kmill
Copy link
Copy Markdown
Contributor

kmill commented Apr 20, 2023

Thanks!

bors r+

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Apr 20, 2023
@kim-em
Copy link
Copy Markdown
Contributor Author

kim-em commented Apr 21, 2023

bors merge

bors bot pushed a commit that referenced this pull request Apr 21, 2023
We had some unfortunate spaghetti code in `solve_by_elim`. When @hrmacbeth had requested additional features for `apply_rules`, the easiest way to provide them was to re-use `solve_by_elim`'s parsing and lemma handling. (See #856.) However `apply_rules` doesn't to backtracking, and `solve_by_elim` is all about it. At the time, `solve_by_elim` didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in `apply_rules`). This PR does that.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
@bors
Copy link
Copy Markdown

bors bot commented Apr 21, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title chore: simplify how solve_by_elim works when not backtracking [Merged by Bors] - chore: simplify how solve_by_elim works when not backtracking Apr 21, 2023
@bors bors bot closed this Apr 21, 2023
@bors bors bot deleted the simplify_backtracking_solve_by_elim branch April 21, 2023 04:11
kbuzzard pushed a commit that referenced this pull request Apr 22, 2023
We had some unfortunate spaghetti code in `solve_by_elim`. When @hrmacbeth had requested additional features for `apply_rules`, the easiest way to provide them was to re-use `solve_by_elim`'s parsing and lemma handling. (See #856.) However `apply_rules` doesn't to backtracking, and `solve_by_elim` is all about it. At the time, `solve_by_elim` didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in `apply_rules`). This PR does that.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
kim-em added a commit that referenced this pull request May 10, 2023
We had some unfortunate spaghetti code in `solve_by_elim`. When @hrmacbeth had requested additional features for `apply_rules`, the easiest way to provide them was to re-use `solve_by_elim`'s parsing and lemma handling. (See #856.) However `apply_rules` doesn't to backtracking, and `solve_by_elim` is all about it. At the time, `solve_by_elim` didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in `apply_rules`). This PR does that.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
hrmacbeth pushed a commit that referenced this pull request May 10, 2023
We had some unfortunate spaghetti code in `solve_by_elim`. When @hrmacbeth had requested additional features for `apply_rules`, the easiest way to provide them was to re-use `solve_by_elim`'s parsing and lemma handling. (See #856.) However `apply_rules` doesn't to backtracking, and `solve_by_elim` is all about it. At the time, `solve_by_elim` didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in `apply_rules`). This PR does that.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
hrmacbeth pushed a commit that referenced this pull request May 11, 2023
We had some unfortunate spaghetti code in `solve_by_elim`. When @hrmacbeth had requested additional features for `apply_rules`, the easiest way to provide them was to re-use `solve_by_elim`'s parsing and lemma handling. (See #856.) However `apply_rules` doesn't to backtracking, and `solve_by_elim` is all about it. At the time, `solve_by_elim` didn't have clean separation between its "lemma application" and "backtracking" considerations, so the solution was to add some hacks the prevented the backtracking from actually occurring, in the backtracking code...

Since #2920, those considerations have been cleanly separated out. Thus it's possible to greatly simplify how we don't backtrack when we don't want to (in `apply_rules`). This PR does that.



Co-authored-by: Scott Morrison <scott.morrison@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants