-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add intelligent fallback from backjump to backtrack #144
Conversation
ee51fdf
to
fb66f3a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
8c8c6d0
to
8ecc834
Compare
This comment was marked as outdated.
This comment was marked as outdated.
FYI a seperate Pip PR (pypa/pip#12459) I have removes all examples I have tested for backjumping incorrectly producing |
8ecc834
to
d2d6f3f
Compare
There was a problem hiding this 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.
33af3a1
to
101aa02
Compare
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. |
101aa02
to
173c71c
Compare
173c71c
to
ab777f1
Compare
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. |
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. |
Fixes #134
Alternative to #142, fixes both the requirments mentioned here #142 (comment)
I beleive this strategy is fairly foolproof:
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!