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

Rework backtracking and state management #62

Merged
merged 5 commits into from
Nov 27, 2020
Merged

Rework backtracking and state management #62

merged 5 commits into from
Nov 27, 2020

Conversation

uranusjr
Copy link
Member

For pypa/pip#9011.

I broke down the change into several commits. The last commit is the real "meat", others are only clean ups and test cases.

Quoting from the commit message:

The "forward" part of resolution now works like this:

  • A "root state" is populated. This state does not produce a "pin" (decide on a candidate to satisfy a requirement), but only contain the root requirements, so subsequence rounds can use it as basis.
  • Each subsequent round pushes a state, pin a candidate on it for it, and check whether the pin results in a conflict.
  • If no conflcits occur, this round ends, and a new round is started. This happens until either all sets are pinned.

When a conflict happen, the backtracking process discards the current round's state, and try to modify the previous state so the conflict can be avoided. If this modification is unsuccessful, the still previous
state is tried, until a workable state is found (or we run out of choices).

This makes each round and its associating state's lifeline easier to reason with. Each successful forward round would create a state with a pin in the stack. A round ending with backtracking will not result in a new state in the stack, but have a previously-pinned state unpinned (and modified), with everything newer than it removed.

@uranusjr uranusjr mentioned this pull request Nov 27, 2020
Copy link
Contributor

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Appreciate the comments added in! I'd been struggling to follow the code flow earlier, and these comments are immensely helpful!

Copy link
Collaborator

@pfmoore pfmoore left a comment

Choose a reason for hiding this comment

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

I agree the comments make the logic a lot easier to follow. The new process looks good to me, I like the fact that each round the invariant is that the top stack item is in an unpinned state, that makes the reasoning much easier to follow.

The new structure gives me much more confidence that the logic is correct 🙂

The "forward" part of resolution now works like this:

* A "root state" is populated. This state does not produce a "pin" but
  only populate the root requirements, so subsequence rounds can use it
  as the basis.
* Each subsequent round pushes a state, pin a candidate for it, and
  check whether the pin results in a conflict.
* If no conflcits occur, the round ends, and a new round is started.

When a conflict happens, the backtracking process discards the current
round's state, and try to modify the previous state so the conflict can
be avoided. If this modification is unsuccessful, the still previous
state is tried, until a workable state is found (or we run out of
things to try).

This makes each round and its associating state's lifeline easier to
reason with. Each successful forward round would create a state with a
pin in the stack. A round ending with backtracking will not result in a
new state in the stack, but have a previously-pinned state unpinned
(and modified), with everything newer than it removed.
@uranusjr uranusjr merged commit 10f3521 into master Nov 27, 2020
@uranusjr uranusjr deleted the backtrack-redo branch November 27, 2020 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants