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

A faster Installed-First policy #55

Closed
wants to merge 35 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2e5c8b2
ENH: Add a priority queue and smart assignment dict
johntyree Oct 31, 2015
d480bac
ENH: Use AssignmentSet in MiniSAT
johntyree Oct 31, 2015
c5db343
TST: update tests for AssignmentSet class
johntyree Nov 3, 2015
3326993
ENH: add pqueue based policy
johntyree Oct 31, 2015
f31e69a
ENH: add required packages by id
johntyree Oct 31, 2015
78933d3
MAINT: add comments to tricky policy methods
johntyree Oct 31, 2015
2c89732
ENH: only compute a single pkg id by default
johntyree Oct 31, 2015
4068290
ENH: add policy wrapper class for logging
johntyree Oct 31, 2015
f6dce8f
ENH: Use new version of InstalledFirstPolicy
johntyree Oct 31, 2015
ea1396b
TST: helper function for viewing pkg set deltas
johntyree Oct 31, 2015
8413695
MAINT: shorten overly-long line
johntyree Oct 31, 2015
ebf8e24
ENH: When peeking, only push-pop if necessary
johntyree Oct 31, 2015
60cf276
REF: split PriorityQueue into separate module
johntyree Oct 31, 2015
d7ccf4f
BUG: required/installed pkgs must be ordered by version
johntyree Oct 31, 2015
a1e4c40
BUG: always update queue if priority changes
johntyree Oct 31, 2015
bf895b8
TST: reenable crashing test case
johntyree Oct 31, 2015
c2668ce
TST: add helpful debug functions for test inspection
johntyree Oct 31, 2015
d6aae02
MAINT: adhere to IPolicy interface
johntyree Nov 3, 2015
76a52f9
BUG: adjust num_assigned when deleting items
johntyree Nov 3, 2015
5a995fe
BUG: return python2-typed values from AssignmentSet
johntyree Nov 3, 2015
06c6499
ENH: support peeking at the changelog
johntyree Nov 3, 2015
4483b54
TST: add tests for AssignmentSet
johntyree Nov 3, 2015
dca7a6c
TST: mark iris as failure until #54 is merged
johntyree Nov 3, 2015
b676d9c
REF: use a simpler scheme for priorities
johntyree Nov 3, 2015
d9e1c01
BUG: don't ignore empty assignment dict
johntyree Nov 3, 2015
7b5b790
ENH: only check to update priorities once
johntyree Nov 3, 2015
60587c2
DOC: clarify the behavior of PriorityQueue.peek()
johntyree Nov 3, 2015
6c957bf
TST: tasks in the queue can only appear once
johntyree Nov 3, 2015
537d39a
TST: ensure tests are deterministic
johntyree Nov 3, 2015
ac3de16
BUG: maintain deterministic iteration order over AssignmentSet
johntyree Nov 3, 2015
7a369b5
ENH: speed up changelog
johntyree Nov 4, 2015
158c316
BUG: there should be no 0 literal
johntyree Nov 4, 2015
a20d9b0
ENH: compute literal value in AssignmentSet
johntyree Nov 4, 2015
5015ae4
MAINT: explain some convoluted code
johntyree Nov 4, 2015
1dba7bf
ENH: compute num_assigned from _assigned_literals set
johntyree Nov 4, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions simplesat/dependency_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,7 @@ def _create_rules(self, request):
pool.package_id(package)
for package in pool.what_provides(requirement)
]
self._policy.add_packages_by_id(requirement_ids)

# Add installed packages.
self._policy.add_packages_by_id(
[pool.package_id(package) for package in installed_repository]
)
self._policy.add_requirements(requirement_ids)

installed_map = collections.OrderedDict()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add_requirements now implies hard requirements, not just packages we should prefer if possible. The Policy gets a reference to installed_repository and can figure out on its own which packages are currently installed if it wants to.

Copy link
Contributor

Choose a reason for hiding this comment

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

from an API POV, I wonder if it would not be simpler to remove add_packages_by_id altogether, and make it an implementation detail (called at creation time from a passed installed_repository).

for package in installed_repository:
Expand Down
113 changes: 113 additions & 0 deletions simplesat/sat/assignment_set.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-


import six

from collections import OrderedDict
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: let's put stdlib import first



class _MISSING(object):
pass
MISSING = _MISSING()


class AssignmentSet(object):

"""A collection of literals and their assignments."""

def __init__(self, assignments=None):
# Changelog is a dict of id -> (original value, new value)
# FIXME: Verify that we really need ordering here
self._data = OrderedDict()
self._orig = {}
self._cached_changelog = None
self._assigned_literals = set()
for k, v in (assignments or {}).items():
self[k] = v

def __setitem__(self, key, value):
assert key > 0

prev_value = self.get(key)

if prev_value is not None:
self._assigned_literals.difference_update((key, -key))

if value is not None:
self._assigned_literals.add(key if value else -key)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I understand that code: if value is not None, the if inside in the add is redundant, right (value cannot be 0 I assume ?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Consequently, assuming the above comment is accurate, when does _assigned_literals get negative keys ?


self._update_diff(key, value)
self._data[key] = value

def __delitem__(self, key):
self._update_diff(key, MISSING)
prev = self._data.pop(key)
if prev is not None:
self._assigned_literals.difference_update((key, -key))

def __getitem__(self, key):
return self._data[key]

def get(self, key, default=None):
return self._data.get(key, default)

def __len__(self):
return len(self._data)

def __contains__(self, key):
return key in self._data

def items(self):
return list(self._data.items())

def iteritems(self):
return six.iteritems(self._data)

def keys(self):
return list(self._data.keys())

def values(self):
return list(self._data.values())

def _update_diff(self, key, value):
prev = self._data.get(key, MISSING)
self._orig.setdefault(key, prev)
# If a value changes, dump the cached changelog
self._cached_changelog = None

def get_changelog(self):
if self._cached_changelog is None:
self._cached_changelog = {
key: (old, new)
for key, old in six.iteritems(self._orig)
for new in [self._data.get(key, MISSING)]
if new != old
}
return self._cached_changelog

def consume_changelog(self):
old = self.get_changelog()
self._orig = {}
self._cached_changelog = {}
return old

def copy(self):
new = AssignmentSet()
new._data = self._data.copy()
new._orig = self._orig.copy()
new._assigned_literals = self._assigned_literals.copy()
return new

def value(self, lit):
""" Return the value of literal in terms of the positive. """
if lit in self._assigned_literals:
return True
elif -lit in self._assigned_literals:
return False
else:
return None

@property
def num_assigned(self):
return len(self._assigned_literals)
7 changes: 3 additions & 4 deletions simplesat/sat/clause.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from collections import OrderedDict

from .utils import value


class Constraint(object):
pass
Expand All @@ -13,6 +11,7 @@ class Clause(Constraint):

def __init__(self, lits, learned=False):
self.learned = learned
# This maintains the ordering while removing duplicate values
self.lits = list(OrderedDict.fromkeys(lits).keys())

def rewatch(self, assignments, lit):
Expand Down Expand Up @@ -41,15 +40,15 @@ def rewatch(self, assignments, lit):
if lits[0] == -lit:
lits[0], lits[1] = lits[1], -lit

if value(lits[0], assignments) is True:
if assignments.value(lits[0]) is True:
# This clause has been satisfied, add it back to the watch list. No
# unit information can be deduced.
return None

# Look for another literal to watch, and switch it with lit[1] to keep
# the assumption on the watched literals in place.
for n, other in enumerate(lits[2:]):
if value(other, assignments) is not False:
if assignments.value(other) is not False:
# Found a new literal that could serve as a watch.
lits[1], lits[n + 2] = other, -lit
return None
Expand Down
14 changes: 6 additions & 8 deletions simplesat/sat/minisat.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

from six.moves import range

from .assignment_set import AssignmentSet
from .clause import Clause
from .policy import DefaultPolicy
from .utils import value


class MiniSATSolver(object):
Expand Down Expand Up @@ -41,7 +41,7 @@ def __init__(self, policy=None):
self.clauses = []
self.watches = defaultdict(list)

self.assignments = OrderedDict()
self.assignments = AssignmentSet()

# A list of literals which become successively true (because of direct
# assignment, or by unit propagation).
Expand All @@ -64,7 +64,6 @@ def __init__(self, policy=None):
# Whether the system is satisfiable.
self.status = None

#
self._policy = policy or DefaultPolicy()

def add_clause(self, clause):
Expand Down Expand Up @@ -116,7 +115,7 @@ def propagate(self):
if unit is not None:
# TODO Refactor this to take into account the return value
# of enqueue().
if value(unit, self.assignments) is False:
if self.assignments.value(unit) is False:
# Conflict. Clear the queue and re-insert the remaining
# unwatched clauses into the watch list.
self.prop_queue.clear()
Expand All @@ -130,7 +129,7 @@ def propagate(self):
def enqueue(self, lit, cause=None):
""" Enqueue a new true literal.
"""
status = value(lit, self.assignments)
status = self.assignments.value(lit)
if status is not None:
# Known fact. Don't enqueue, but return whether this fact
# contradicts the earlier assignment.
Expand Down Expand Up @@ -176,7 +175,7 @@ def search(self):
def validate(self, solution_map):
"""Check whether a given set of assignments solves this SAT problem.
"""
solution_literals = {(+1 if status else -1) * variable
solution_literals = {variable if status else -variable
for variable, status in solution_map.items()}
for clause in self.clauses:
if len(set(clause.lits) & solution_literals) == 0:
Expand Down Expand Up @@ -285,8 +284,7 @@ def assume(self, lit):
def number_assigned(self):
""" Return the number of currently assigned variables.
"""
return len([value for value in self.assignments.values()
if value is not None])
return self.assignments.num_assigned

@property
def number_variables(self):
Expand Down
Loading