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

Add intelligent fallback from backjump to backtrack #144

Closed

Conversation

notatallshaw
Copy link
Contributor

@notatallshaw notatallshaw commented Nov 29, 2023

Fixes #134

Alternative to #142, fixes both the requirments mentioned here #142 (comment)

I beleive this strategy is fairly foolproof:

  1. When backjumping make a backup of the states the first time backjumping skips over states further than backtracking would
  2. If backjumping reaches resolution impossible and a backup state exists, fall back to that state and start backtracking

However, there are downsides, it could definetly cause a valid ResolutionImpossible to take much longer to produce. I have therefore give the calling library a choice on what to do, backjump with backtrack fallback, backjump only, or backtrack only.

If you want tests I need some small guidance, I don't really understand this repos tests.

Let me know what you think!

@notatallshaw

This comment was marked as outdated.

@notatallshaw

This comment was marked as outdated.

@notatallshaw
Copy link
Contributor Author

FYI a seperate Pip PR (pypa/pip#12459) I have removes all examples I have tested for backjumping incorrectly producing ResolutionImpossible, I think it's important to still remove or have a fallback for backjumping, I have a more detailed comment in the alternative PR #142 (comment)

Copy link
Member

@frostming frostming left a comment

Choose a reason for hiding this comment

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

The implementation is a bit convoluted, need more comments to help successors understand what is happening.

@notatallshaw notatallshaw force-pushed the backjump-intelligent-fallback branch 2 times, most recently from 33af3a1 to 101aa02 Compare February 10, 2024 19:04
@notatallshaw
Copy link
Contributor Author

The implementation is a bit convoluted, need more comments to help successors understand what is happening.

I think to a certain extent that is the nature of this solution, it backs up states and only uses those backed up states under the right conditions.

But I have tried to improve the situation breaking up the code a little, adding more comments, and trying to pick better names. Let me know what you think.

@notatallshaw
Copy link
Contributor Author

An further thoughts on this?

It would be good to fix this backjump issue somehow, as it breaks real world requirements for both pip and PDM.

@notatallshaw
Copy link
Contributor Author

I am unhappy with this solution, I am working further on understanding the root cause of the problem: #134 (comment)

I may recreate this PR once I have a working test.

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.

An example of resolution failure while it does have a valid resolution.
3 participants