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.

Sign up for GitHub

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

Performance tweaks #95

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Performance tweaks #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e588234
Trivial tweaks to improve performance
daveraja Mar 7, 2022
133a2d3
Use Predicate template for __bool__ and __len__
daveraja Mar 7, 2022
19c31d1
Calculate the cached hash value for a fact at creation time
daveraja Mar 7, 2022
ee82644
Add a fact's sign to hash generation
daveraja Mar 9, 2022
931d326
Comparison operators for Predicate map directly to the Symbol object
daveraja Apr 1, 2022
56ea86e
Getting FactBase to work with new Predicate semantics
daveraja Apr 4, 2022
aeb1832
Passing current unit tests, but need to add more FactBase tests.
daveraja Apr 4, 2022
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
162 changes: 91 additions & 71 deletions clorm/orm/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
from .templating import (expand_template, PREDICATE_TEMPLATE,
CHECK_SIGN_TEMPLATE, CHECK_SIGN_UNIFY_TEMPLATE,
NO_DEFAULTS_TEMPLATE, ASSIGN_DEFAULT_TEMPLATE,
ASSIGN_COMPLEX_TEMPLATE, PREDICATE_UNIFY_DOCSTRING)
ASSIGN_COMPLEX_TEMPLATE, PREDICATE_UNIFY_DOCSTRING,
PREDICATE_BOOL_DOCSTRING, PREDICATE_LEN_DOCSTRING)

from typing import ( Any, Callable, Dict, Iterator, List, Optional, Sequence, Tuple,
Type, TypeVar, Union, overload, cast )
Expand Down Expand Up @@ -87,6 +88,7 @@ def get_args(t: Type[Any]) -> Tuple[Any, ...]:
__all__ = [
'ClormError',
'Comparator',
'Placeholder',
'BaseField',
'Raw',
'RawField',
Expand Down Expand Up @@ -285,8 +287,35 @@ def _wqc(a):
except:
return str(_wsce(a))


#------------------------------------------------------------------------------
# Placeholder allows for variable substituion of a query. Placeholder is
# an abstract class that exposes no API other than its existence.
# ------------------------------------------------------------------------------
# ------------------------------------------------------------------------------
class Placeholder(abc.ABC):

r"""An abstract class for defining parameterised queries.

Currently, Clorm supports 4 placeholders: ph1\_, ph2\_, ph3\_, ph4\_. These
correspond to the positional arguments of the query execute function call.

"""
def __init__(self):
self._cmplx = None

@abc.abstractmethod
def __eq__(self, other): pass

@abc.abstractmethod
def __hash__(self): pass

@abc.abstractmethod
def make_complex_tuple_clone(self, cmplx):
pass

@abc.abstractmethod
def ground(self, ctx, *args, **kwargs):
pass

# ------------------------------------------------------------------------------
#
Expand Down Expand Up @@ -457,6 +486,27 @@ def notin_(path, seq):
'''Return a query operator to test non-membership of an item in a collection'''
return QCondition(notcontains, seq, path)


# ------------------------------------------------------------------------------
# A Helper function that attaches the complex tuple type if the QCondition path
# is for a complex tuple field
# ------------------------------------------------------------------------------

def _qcondition_tuple_value(ppath, value):
if isinstance(value, PredicatePath):
return value
field = ppath.meta.field
if field is None:
return value
cmplx = field.complex
if cmplx is None:
return value
if not cmplx.meta.is_tuple:
return value
if not isinstance(value, Placeholder):
return cmplx(*value)
return value.make_complex_tuple_clone(cmplx)

#------------------------------------------------------------------------------
# PredicatePath class and supporting metaclass and functions. The PredicatePath
# is crucial to the Clorm API because it implements the intuitive syntax for
Expand Down Expand Up @@ -821,17 +871,17 @@ def __getitem__(self, key):
# Overload the boolean operators to return a functor
#--------------------------------------------------------------------------
def __eq__(self, other):
return QCondition(operator.eq, self, other)
return QCondition(operator.eq, self, _qcondition_tuple_value(self, other))
def __ne__(self, other):
return QCondition(operator.ne, self, other)
return QCondition(operator.ne, self, _qcondition_tuple_value(self, other))
def __lt__(self, other):
return QCondition(operator.lt, self, other)
return QCondition(operator.lt, self, _qcondition_tuple_value(self, other))
def __le__(self, other):
return QCondition(operator.le, self, other)
return QCondition(operator.le, self, _qcondition_tuple_value(self, other))
def __gt__(self, other):
return QCondition(operator.gt, self, other)
return QCondition(operator.gt, self, _qcondition_tuple_value(self, other))
def __ge__(self, other):
return QCondition(operator.ge, self, other)
return QCondition(operator.ge, self, _qcondition_tuple_value(self, other))

#--------------------------------------------------------------------------
# Overload the bitwise operators to catch user-mistakes
Expand Down Expand Up @@ -1021,7 +1071,6 @@ def getpath():
if cleanpath is None: return None
return cleanpath.meta.dealiased


#------------------------------------------------------------------------------
# Helper function to check if a second set of keys is a subset of a first
# set. If it is not it returns the unrecognised keys. Useful for checking a
Expand Down Expand Up @@ -2296,7 +2345,6 @@ def alias(self, name=None):
name = "({}.alias.{})".format(classname,uuid.uuid4().hex)
return self._path_class([PathIdentity(self._parent_cls,name)])


def __len__(self):
'''Returns the number of fields'''
return len(self._byidx)
Expand Down Expand Up @@ -2394,9 +2442,10 @@ def _generate_dynamic_predicate_functions(class_name: str, namespace: Dict):
"sign_check_unify": sign_check_unify,
"args_cltopy": args_cltopy}

template = PREDICATE_TEMPLATE.format(pdefn=pdefn)
bool_status = not pdefn.is_tuple or len(pdefn) > 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this by intention that you have changed the bool expression? Before we have just checked for the length

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes the change was intentional. I was trying to think what was most intuitive from a user perspective.

Most importantly, I think clorm tuples should behave like a normal tuple as much as possible. So an empty tuple should evaluate to false.

But for a normal fact, even one with no parameters, I'm not sure it makes sense for it to evaluate to false. I think the following would be unintuitive:

class P(Predicate):
    pass

p = P()
if not p: 
   print("Feels strange")

Anyway, that was the rationale :)

In any case, It's a bit of a boundary case to define predicates with no parameters and clorm is not well suited to this use case. It's not very efficient or intuitive (every P instance is equivalent). Ideally it would be better to have a simpler mechanism for representing these cases. I just haven't come up with something that could also be made to fit nicely with the rest of clorm; unifiction, FactBases and the query stuff.

template = PREDICATE_TEMPLATE.format(pdefn=pdefn, bool_status=bool_status)
predicate_functions = expand_template(template, **expansions)
# print(f"INIT:\n\n{predicate_functions}\n\n")
# print(f"INIT:\n\n{predicate_functions}\n\n")

ldict = {}
exec(predicate_functions, gdict, ldict)
Expand All @@ -2408,10 +2457,17 @@ def _generate_dynamic_predicate_functions(class_name: str, namespace: Dict):
predicate_unify = ldict["_unify"]
predicate_unify.__name__ = "_unify"
predicate_unify.__doc__ = PREDICATE_UNIFY_DOCSTRING
predicate_bool = ldict["__bool__"]
predicate_bool.__name__ = "__bool__"
predicate_bool.__doc__ = PREDICATE_BOOL_DOCSTRING
predicate_len = ldict["__len__"]
predicate_len.__name__ = "__len__"
predicate_len.__doc__ = PREDICATE_LEN_DOCSTRING

namespace["__init__"] = predicate_init
namespace["_unify"] = predicate_unify

namespace["__bool__"] = predicate_bool
namespace["__len__"] = predicate_len


#------------------------------------------------------------------------------
Expand Down Expand Up @@ -2877,94 +2933,58 @@ def __iter__(self) -> Iterator[PredicatePath]:

def __getitem__(self, idx):
"""Allows for index based access to field elements."""
return self.meta[idx].__get__(self)
return self._meta[idx].__get__(self)

def __bool__(self):
'''Behaves like a tuple: returns False if the predicate/complex-term has no elements'''
return len(self.meta) > 0

def __len__(self):
'''Returns the number of fields in the object'''
return len(self.meta)
### NOTE: __bool__() and __len__() are generated dynamically with
### __init__().

#--------------------------------------------------------------------------
# Overload the unary minus operator to return the complement of this literal
# (if its positive return a negative equivaent and vice-versa)
# --------------------------------------------------------------------------
def __neg__(self):
return self.clone(sign=not self.sign)
return self.clone(sign=not self._sign)

#--------------------------------------------------------------------------
# Overloaded operators
#--------------------------------------------------------------------------
def __eq__(self, other):
"""Overloaded boolean operator."""
if isinstance(other, self.__class__):
return self._field_values == other._field_values and \
self._sign == other._sign
if self.meta.is_tuple:
return self._field_values == other
elif isinstance(other, Predicate):
return False
if isinstance(other, Predicate):
return self._raw == other._raw
return NotImplemented

def __lt__(self, other):
"""Overloaded boolean operator."""

# If it is the same predicate class then compare the sign and fields
if isinstance(other, self.__class__):

# Negative literals are less than positive literals
if self.sign != other.sign: return self.sign < other.sign

return self._field_values < other._field_values

# If different predicates then compare the raw value
elif isinstance(other, Predicate):
return self.raw < other.raw

# Else an error
if isinstance(other, Predicate):
return self._raw < other._raw
return NotImplemented

def __ge__(self, other):
def __le__(self, other):
"""Overloaded boolean operator."""
result = self.__lt__(other)
if result is NotImplemented: return NotImplemented
return not result
if isinstance(other, Predicate):
return self._raw <= other._raw
return NotImplemented

def __gt__(self, other):
"""Overloaded boolean operator."""

# If it is the same predicate class then compare the sign and fields
if isinstance(other, self.__class__):
# Positive literals are greater than negative literals
if self.sign != other.sign: return self.sign > other.sign

return self._field_values > other._field_values

# If different predicates then compare the raw value
if not isinstance(other, Predicate):
return self.raw > other.raw

# Else an error
if isinstance(other, Predicate):
return self._raw > other._raw
return NotImplemented

def __le__(self, other):
def __ge__(self, other):
"""Overloaded boolean operator."""
result = self.__gt__(other)
if result is NotImplemented: return NotImplemented
return not result
if isinstance(other, Predicate):
return self._raw >= other._raw
return NotImplemented

def __hash__(self):
if self._hash is None:
if self.meta.is_tuple: self._hash = hash(self._field_values)
else: self._hash = hash((self.meta.name,self._field_values))
return self._hash

def __str__(self):
"""Returns the Predicate as the string representation of an ASP fact.
"""
return str(self.raw)
return str(self._raw)

def __repr__(self):
return self.__str__()
Expand All @@ -2974,14 +2994,14 @@ def __getstate__(self):
'_sign' : self._sign}

def __setstate__(self, newstate):
self._hash = None
self._field_values = newstate["_field_values"]
self._sign = newstate["_sign"]

clingoargs=[]
for f,v in zip(self.meta, self._field_values):
for f,v in zip(self._meta, self._field_values):
clingoargs.append(v.symbol if f.defn.complex else f.defn.pytocl(v))
self._raw = Function(self.meta.name, clingoargs, self._sign)
self._raw = Function(self._meta.name, clingoargs, self._sign)
self._hash = hash(self._raw)


#------------------------------------------------------------------------------
Expand Down
14 changes: 11 additions & 3 deletions clorm/orm/factbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ class FactBase(object):
and allows for certain fields to be indexed in order to perform more
efficient queries.

The initaliser can be given a collection of predicates. If it is passed
another FactBase then it simply makes a copy (including the indexed fields).
The initialiser can be given a collection of predicates. If it is passed
another FactBase then it simply makes a copy (including the indexed
fields).

FactBase also has a special mode when it is passed a functor instead of a
collection. In this case it performs a delayed initialisation. This means
Expand Down Expand Up @@ -178,6 +179,9 @@ def _init(self, facts=None, indexes=None):
indexes = facts.indexes
if indexes is None: indexes=[]

# Create a set to store all facts
self._facts = set()

# Create FactMaps for the predicate types with indexed fields
grouped = {}

Expand All @@ -200,6 +204,8 @@ def _check_init(self):

def _add(self, arg: Union[Predicate, Iterable[Predicate]]) -> None:
if isinstance(arg, Predicate):
if arg in self._facts:
return None
ptype = arg.__class__
if not ptype in self._factmaps:
self._factmaps[ptype] = FactMap(ptype)
Expand All @@ -208,7 +214,9 @@ def _add(self, arg: Union[Predicate, Iterable[Predicate]]) -> None:
if isinstance(arg, str) or not isinstance(arg, Iterable):
raise TypeError(f"'{arg}' is not a Predicate instance")

sorted_facts = sorted(arg, key=lambda x: x.__class__.__name__)
sorted_facts = sorted(filter(lambda f: f not in self._facts, arg),
key=lambda x: x.__class__.__name__)
self._facts.update(sorted_facts)
for ptype, grouped_facts in itertools.groupby(sorted_facts, lambda x: x.__class__):
if not issubclass(ptype, Predicate):
raise TypeError(f"{list(grouped_facts)} are not Predicate instances")
Expand Down
Loading