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

Implement backjumping over unrelated states #113

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

bennati
Copy link
Contributor

@bennati bennati commented Dec 23, 2022

The current backtracking logic assumes that the last state in the stack
is the state that caused the incompatibility. This is not always the case.
For example, given three requirements A, B and C, with dependencies
A1, B1 and C1, where A1 and B1 are incompatible.
The requirements are processed one after the other, so the last state
is related to C, while the incompatibility is caused by B.

The current behavior causes significant slowdowns in case there are
many candidates for B and C, as all their combination are evaluated
before a compatible version of B can be found.

The new behavior discards a state if the packages that cause the
incompatibility are not found among the direct dependencies
of the candidate in the current state.
In our example, this causes the state related to C to be dropped
without evaluating any of its candidates, until a compatible candidate
of B is found.

This change can be tested with the following requirements:

boto3==1.10.16
s3fs
seaborn

@pombredanne
Copy link

This is an awesome finding!

@pombredanne
Copy link

@bennati Could you add a test?

@bennati bennati changed the title fix infinite loop for resolvable conflicts. Draft: fix infinite loop for resolvable conflicts. Dec 24, 2022
@bennati bennati force-pushed the fix-infinite-loop-resolution branch from f18552a to 92d229b Compare January 3, 2023 16:18
@bennati
Copy link
Contributor Author

bennati commented Jan 3, 2023

I realized the error is something else entirely, please review again :)

@bennati bennati changed the title Draft: fix infinite loop for resolvable conflicts. Draft: Speed up backtracking by skipping unrelated states. Jan 3, 2023
@bennati bennati requested a review from uranusjr January 3, 2023 16:19
@bennati bennati force-pushed the fix-infinite-loop-resolution branch 2 times, most recently from 2f15247 to d190f08 Compare January 3, 2023 16:45
@pradyunsg
Copy link
Contributor

If the change does what the message is implying, I'd prefer that we rename the function to _backjump :)

@bennati
Copy link
Contributor Author

bennati commented Jan 4, 2023

Why do you want to rename the function? The function is not jumping anywhere, it is backtracking states in order until it reaches the correct state.

@bennati
Copy link
Contributor Author

bennati commented Jan 4, 2023

@pombredanne can you elaborate on what kind test you would like to have?
Basing the test on the time it takes to process a requirements file does not seem ideal.

@pradyunsg
Copy link
Contributor

pradyunsg commented Jan 4, 2023

https://en.m.wikipedia.org/wiki/Backjumping is why I've suggested the name that I suggested. :)

src/resolvelib/resolvers.py Outdated Show resolved Hide resolved
src/resolvelib/resolvers.py Outdated Show resolved Hide resolved
src/resolvelib/resolvers.py Outdated Show resolved Hide resolved
src/resolvelib/resolvers.py Show resolved Hide resolved
src/resolvelib/resolvers.py Outdated Show resolved Hide resolved
src/resolvelib/resolvers.py Outdated Show resolved Hide resolved
@bennati bennati requested review from frostming and uranusjr and removed request for uranusjr and frostming January 11, 2023 09:24
@bennati bennati force-pushed the fix-infinite-loop-resolution branch 2 times, most recently from 66c76df to 0fa102a Compare January 12, 2023 15:22
@bennati
Copy link
Contributor Author

bennati commented Jan 12, 2023

The Lint CI fails due to "unused" imports and variables, but removing them causes the next step in the CI (mypy) to fail because the same imports and variables are not defined.
Not sure how to solve these issues.

@bennati bennati requested review from frostming and uranusjr and removed request for uranusjr and frostming January 13, 2023 08:10
@uranusjr
Copy link
Member

You can use TYPE_CHECKING to conditionally import those, so they are only imported for type checking.

@pradyunsg
Copy link
Contributor

pradyunsg commented Feb 6, 2023

Actually, @notatallshaw the issue you're reporting here seems to have been introduced in 0.9.0 -- I've just reproduced that in pip + resolvelib 0.9.0 in addition to this branch. My understanding is that #111 is where we introduced the change (which is trimming information aggresively).

@pradyunsg
Copy link
Contributor

If this is not too much to ask making a release candidate would help a lot testing downstream.

Yes, and, you can do pip install https://github.com/sarugaku/resolvelib/archive/refs/heads/main.zip if you really want the latest and greatest. :)

@notatallshaw
Copy link
Collaborator

Actually, @notatallshaw the issue you're reporting here seems to have been introduced in 0.9.0 -- I've just reproduced that in pip + resolvelib 0.9.0 in addition to this branch. My understanding is that #111 is where we introduced the change (which is trimming information aggresively).

Agreed, I've opened a new issue with a reproducible set of steps: #120

bennati added a commit to bennati/python-inspector that referenced this pull request Feb 7, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Feb 7, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#116

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Feb 7, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Feb 7, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Feb 23, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
@frostming
Copy link
Member

Hi, @pradyunsg @uranusjr do you have any preference on when a new release will be out with this fix?

@uranusjr
Copy link
Member

uranusjr commented Mar 7, 2023

Since the associated tests are done I think I’ll cut a release. I think this is also a good chance to go 1.0 as well? I’ll wait 24 hours in case anyone objects.

@pradyunsg
Copy link
Contributor

Go ahead and cut the release. :)

@uranusjr
Copy link
Member

uranusjr commented Mar 8, 2023

Still a few hours to 24 but I’ll call it.

bennati added a commit to bennati/python-inspector that referenced this pull request Mar 17, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Mar 17, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
bennati added a commit to bennati/python-inspector that referenced this pull request Mar 17, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
gignsky added a commit to gignsky/tdarr-node-switcher that referenced this pull request May 24, 2023
Bumps [resolvelib](https://github.com/sarugaku/resolvelib) from 0.8.1 to
1.0.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/sarugaku/resolvelib/blob/main/CHANGELOG.rst">resolvelib's
changelog</a>.</em></p>
<blockquote>
<h1>1.0.1 (2023-03-09)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Fix calls to opaque objects and use provider interface calls
instead. <code>[#126](sarugaku/resolvelib#126)
&lt;https://github.com/sarugaku/resolvelib/issues/126&gt;</code>_</li>
</ul>
<h1>1.0.0 (2023-03-08)</h1>
<h2>Features</h2>
<ul>
<li>Implement backjumping to significantly speed up the resolution
process by skipping over irrelevant parts of the resolution search
space. <code>[#113](sarugaku/resolvelib#113)
&lt;https://github.com/sarugaku/resolvelib/issues/113&gt;</code>_</li>
</ul>
<h1>0.9.0 (2022-11-17)</h1>
<h2>Features</h2>
<ul>
<li>A new reporter hook <code>rejecting_candidate</code> is added,
replacing <code>backtracking</code>.
The hook is called every time the resolver rejects a conflicting
candidate before
trying out the next one in line.
<code>[#101](sarugaku/resolvelib#101)
&lt;https://github.com/sarugaku/resolvelib/issues/101&gt;</code>_</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Some valid states that were previously rejected are now accepted.
This affects
states where multiple candidates for the same dependency conflict with
each
other. The <code>information</code> argument passed to
<code>AbstractProvider.get_preference</code> may now contain empty
iterators. This has
always been allowed by the method definition but it was previously not
possible
in practice.
<code>[#91](sarugaku/resolvelib#91)
&lt;https://github.com/sarugaku/resolvelib/issues/91&gt;</code>_</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/c9ef371ad96e698bf3e0bb09acc682bd43e39bd7"><code>c9ef371</code></a>
Release 1.0.1</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/fb97e4c71fe520ae6d1704b7be8bd742430cd6e6"><code>fb97e4c</code></a>
Add missing news fragment for patch release</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/34bb1a7474e6a45103eeb08d9bbfd1a48e1b1cd3"><code>34bb1a7</code></a>
Merge pull request <a
href="https://redirect.github.com/sarugaku/resolvelib/issues/127">#127</a>
from sarugaku/avoid-intermediate-set</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/409bcf753074ba5b03ffd259511bf12b5d181fde"><code>409bcf7</code></a>
Use itertools.chain to avoid intermediate set</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/e8fecf776aafab9104eb0d101c4de08ce3c37eaf"><code>e8fecf7</code></a>
Merge pull request <a
href="https://redirect.github.com/sarugaku/resolvelib/issues/126">#126</a>
from sarugaku/fix-</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/5ede4c4bd205e5c61d339ba957deb44c4f5400f9"><code>5ede4c4</code></a>
use set comprehension</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/7e8adca961b96c159d3391049e6a6a37e25ed525"><code>7e8adca</code></a>
fix: use the protocol method instead of .name attribute</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/02fb73619d536b14b621ecddb98ad24ccd6093d2"><code>02fb736</code></a>
Merge pull request <a
href="https://redirect.github.com/sarugaku/resolvelib/issues/125">#125</a>
from sarugaku/release/1.0</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/69ae3253545a1da31a0096686693eed5c2dee08c"><code>69ae325</code></a>
Update release process</li>
<li><a
href="https://github.com/sarugaku/resolvelib/commit/40c867a2b7d12e3736ccf6c621a44a1b0dbefbc7"><code>40c867a</code></a>
Prebump to 1.0.1.dev0</li>
<li>Additional commits viewable in <a
href="https://github.com/sarugaku/resolvelib/compare/0.8.1...1.0.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=resolvelib&package-manager=pip&previous-version=0.8.1&new-version=1.0.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

You can trigger a rebase of this PR by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>> **Note**
> Automatic rebases have been disabled on this pull request as it has
been open for over 30 days.
gignsky added a commit to gignsky/tdarr-node-switcher that referenced this pull request May 24, 2023
Bumps [resolvelib](https://github.com/sarugaku/resolvelib) from 0.8.1 to
1.0.1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a

href="https://github.com/sarugaku/resolvelib/blob/main/CHANGELOG.rst">resolvelib's
changelog</a>.</em></p>
<blockquote>
<h1>1.0.1 (2023-03-09)</h1>
<h2>Bug Fixes</h2>
<ul>
<li>Fix calls to opaque objects and use provider interface calls
instead. <code>[#126](sarugaku/resolvelib#126)
&lt;https://github.com/sarugaku/resolvelib/issues/126&gt;</code>_</li>
</ul>
<h1>1.0.0 (2023-03-08)</h1>
<h2>Features</h2>
<ul>
<li>Implement backjumping to significantly speed up the resolution
process by skipping over irrelevant parts of the resolution search
space. <code>[#113](sarugaku/resolvelib#113)
&lt;https://github.com/sarugaku/resolvelib/issues/113&gt;</code>_</li>
</ul>
<h1>0.9.0 (2022-11-17)</h1>
<h2>Features</h2>
<ul>
<li>A new reporter hook <code>rejecting_candidate</code> is added,
replacing <code>backtracking</code>.
The hook is called every time the resolver rejects a conflicting
candidate before
trying out the next one in line.
<code>[#101](sarugaku/resolvelib#101)
&lt;https://github.com/sarugaku/resolvelib/issues/101&gt;</code>_</li>
</ul>
<h2>Bug Fixes</h2>
<ul>
<li>Some valid states that were previously rejected are now accepted.
This affects
states where multiple candidates for the same dependency conflict with
each
other. The <code>information</code> argument passed to
<code>AbstractProvider.get_preference</code> may now contain empty
iterators. This has
always been allowed by the method definition but it was previously not
possible
in practice.
<code>[#91](sarugaku/resolvelib#91)
&lt;https://github.com/sarugaku/resolvelib/issues/91&gt;</code>_</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/c9ef371ad96e698bf3e0bb09acc682bd43e39bd7"><code>c9ef371</code></a>
Release 1.0.1</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/fb97e4c71fe520ae6d1704b7be8bd742430cd6e6"><code>fb97e4c</code></a>
Add missing news fragment for patch release</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/34bb1a7474e6a45103eeb08d9bbfd1a48e1b1cd3"><code>34bb1a7</code></a>
Merge pull request <a

href="https://redirect.github.com/sarugaku/resolvelib/issues/127">#127</a>
from sarugaku/avoid-intermediate-set</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/409bcf753074ba5b03ffd259511bf12b5d181fde"><code>409bcf7</code></a>
Use itertools.chain to avoid intermediate set</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/e8fecf776aafab9104eb0d101c4de08ce3c37eaf"><code>e8fecf7</code></a>
Merge pull request <a

href="https://redirect.github.com/sarugaku/resolvelib/issues/126">#126</a>
from sarugaku/fix-</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/5ede4c4bd205e5c61d339ba957deb44c4f5400f9"><code>5ede4c4</code></a>
use set comprehension</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/7e8adca961b96c159d3391049e6a6a37e25ed525"><code>7e8adca</code></a>
fix: use the protocol method instead of .name attribute</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/02fb73619d536b14b621ecddb98ad24ccd6093d2"><code>02fb736</code></a>
Merge pull request <a

href="https://redirect.github.com/sarugaku/resolvelib/issues/125">#125</a>
from sarugaku/release/1.0</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/69ae3253545a1da31a0096686693eed5c2dee08c"><code>69ae325</code></a>
Update release process</li>
<li><a

href="https://github.com/sarugaku/resolvelib/commit/40c867a2b7d12e3736ccf6c621a44a1b0dbefbc7"><code>40c867a</code></a>
Prebump to 1.0.1.dev0</li>
<li>Additional commits viewable in <a

href="https://github.com/sarugaku/resolvelib/compare/0.8.1...1.0.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility

score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=resolvelib&package-manager=pip&previous-version=0.8.1&new-version=1.0.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

You can trigger a rebase of this PR by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>> **Note**
> Automatic rebases have been disabled on this pull request as it has
been open for over 30 days.

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@notatallshaw
Copy link
Collaborator

FYI, I've raised a PR to remove backjumping: #142

bennati added a commit to bennati/python-inspector that referenced this pull request Dec 1, 2023
It includes fix for backjumping, see
sarugaku/resolvelib#113

Relates-to: aboutcode-org#106

Signed-off-by: Bennati, Stefano <[email protected]>
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.

7 participants