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

Introduce priority queue-based policy again #81

Closed
wants to merge 21 commits into from

Conversation

johntyree
Copy link
Contributor

Another remake of #65, #55.

The notes listed in those issues still apply.

An additional trick that this policy does is attempt to minimize the number of suggestions required by suggesting packages at the top of the dependency tree first. This causes many more assignments to be made via unit propagation. For scenarios like epd_full_upgrade this can cut the number of suggestions needed significantly.

Timings are for epd_full_upgrade and should be comparable to #78.

Master

ELAPSED : Generate Rules       : 1.097054e+00
ELAPSED : Solver Init          : 2.741041e-01
ELAPSED : SAT Solve            : 4.645386e+00

This branch

ELAPSED : Generate Rules       : 1.138217e+00
ELAPSED : Solver Init          : 2.549131e-01
ELAPSED : SAT Solve            : 6.652930e-01

@cournape
Copy link
Contributor

Is this PR still relevant following our recent discussions ?

@johntyree
Copy link
Contributor Author

There are a few major differences between this policy and the current UndeterminedClausePolicy.

  • The biggest one is that this policy makes is much easier to have fine-grained control over the order in which packages get suggested. For example, if pillow had started numbering back at 0.1, but we needed to favor pillow over pil it's handled easily here by fiddling with priorities.
  • The second is that by only revisiting the clauses and filling the decision_set when it fully empties, the UndeterminedClausePolicy is implicitly assuming that those decisions are valid forever. This is working out fine so far it seems. In some cases I think it can result in less than optimal packages being selected. I'm not sure of this, however.

This could also be simplified by not sorting packages topologically. It was an experiment that does seem to reduce the number of suggestions required overall, but doesn't affect the quality of the result.

I think this policy has value, but I would also understand not wanting to bring in code that is arguably no longer solving a concrete problem after #78.

@cournape
Copy link
Contributor

Let's keep this opened for now, I will look at it when I have some time

@johntyree
Copy link
Contributor Author

There are probably some parts of this that could be used to improve UndeterminedClausePolicy as well. We're not currently making use of the assignments change log there, for example.

@johntyree
Copy link
Contributor Author

I'm going to split up the policy module to avoid getting conflicts while I juggle this around.

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.

2 participants