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
[MRG] Caching and small run preparation improvements #832
Changes from 6 commits
347620c
ea4e03f
1d286df
b714416
12b1843
d4c3bdc
c0e4850
88dc2ad
011820d
a1bfddc
fa200a9
05c9544
6d16492
a30371f
83ac3e7
fdd50de
049a654
49eef8c
6803457
402de76
5cff0ed
ca1dfb2
3b34b53
fb5d6d7
8633c7b
38b4360
eea3189
ffde29e
dc1378e
b6f9e1e
26c6229
09f83c8
3158274
04b0310
6d47a46
280f8d7
ca0e8a1
a6d74b9
a56484f
407a0f8
b070223
598a74d
4298e47
f921573
404779d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,7 +88,6 @@ def variables_by_owner(variables, owner): | |
return dict([(varname, var) for varname, var in variables.iteritems() | ||
if getattr(var.owner, 'name', None) is owner_name]) | ||
|
||
|
||
class Variable(object): | ||
''' | ||
An object providing information about model variables (including implicit | ||
|
@@ -165,6 +164,16 @@ def __init__(self, name, dimensions=DIMENSIONLESS, owner=None, dtype=None, | |
#: Whether the variable is an array | ||
self.array = array | ||
|
||
#: Mark the list of attributes that should not be considered for caching | ||
#: of state update code, etc. | ||
self._cache_irrelevant_attributes = ['owner'] | ||
|
||
@property | ||
def _state_tuple(self): | ||
return tuple(value for key, value in self.__dict__.iteritems() | ||
if key not in (self._cache_irrelevant_attributes + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to sort the items in a dict or is the order deterministic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It shouldn't matter here since the order is arbitrary but deterministic, but I'm not 100% sure there are no corner cases. So, using |
||
['_cache_irrelevant_attributes'])) | ||
|
||
@property | ||
def is_boolean(self): | ||
return np.issubdtype(np.bool, self.dtype) | ||
|
@@ -274,9 +283,7 @@ def __repr__(self): | |
constant=repr(self.constant), | ||
read_only=repr(self.read_only)) | ||
|
||
_state_tuple = property(lambda self: (self.dim, self.dtype, self.scalar, | ||
self.constant, self.read_only, | ||
self.dynamic, self.name)) | ||
|
||
|
||
# ------------------------------------------------------------------------------ | ||
# Concrete classes derived from `Variable` -- these are the only ones ever | ||
|
@@ -341,11 +348,6 @@ def __init__(self, name, value, dimensions=DIMENSIONLESS, owner=None): | |
def get_value(self): | ||
return self.value | ||
|
||
# In contrast to other `Variable` objects, `Constant` objects are recreated | ||
# for every run, so we cannot simply use their identity to compare them. | ||
_state_tuple = property(lambda self: super(Constant, self)._state_tuple + | ||
(self.value, )) | ||
|
||
|
||
class AuxiliaryVariable(Variable): | ||
''' | ||
|
@@ -473,8 +475,6 @@ def get_addressable_value_with_unit(self, name, group): | |
return VariableView(name=name, variable=self, group=group, | ||
dimensions=self.dim) | ||
|
||
_state_tuple = property(lambda self: super(ArrayVariable, self)._state_tuple + | ||
(self.size, self.unique, id(self.device))) | ||
|
||
class DynamicArrayVariable(ArrayVariable): | ||
''' | ||
|
@@ -559,6 +559,9 @@ def __init__(self, name, owner, size, device, dimensions=DIMENSIONLESS, | |
dynamic=True, | ||
read_only=read_only, | ||
unique=unique) | ||
# The size of a dynamic variable can of course change and changes in | ||
# size should not invalidate the cache | ||
self._cache_irrelevant_attributes += ['size'] | ||
|
||
@property | ||
def dimensions(self): | ||
|
@@ -584,13 +587,6 @@ def resize(self, new_size): | |
|
||
self.size = new_size | ||
|
||
# Note that we inherit the state tuple from Variable, *not* from ArrayVariable. | ||
# Otherwise the size would be part of the tuple, but of course the size of | ||
# a dynamic variable can change. | ||
_state_tuple = property(lambda self: super(ArrayVariable, self)._state_tuple + | ||
(self.unique, id(self.device), self.ndim, | ||
self.resize_along_first, self.needs_reference_update)) | ||
|
||
|
||
class Subexpression(Variable): | ||
''' | ||
|
@@ -670,9 +666,6 @@ def __repr__(self): | |
expr=repr(self.expr), | ||
owner=self.owner.name) | ||
|
||
_state_tuple = property(lambda self: super(Subexpression, self)._state_tuple + | ||
(self.expr, id(self.device))) | ||
|
||
# ------------------------------------------------------------------------------ | ||
# Classes providing views on variables and storing variables information | ||
# ------------------------------------------------------------------------------ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -426,6 +426,18 @@ def __init__(self, type, varname, dimensions, var_type=FLOAT, expr=None, | |
# will be set later in the sort_subexpressions method of Equations | ||
self.update_order = -1 | ||
|
||
#: Mark the list of attributes that should not be considered for caching | ||
#: of state update code, etc. | ||
self._cache_irrelevant_attributes = ['update_order'] | ||
|
||
@property | ||
def _state_tuple(self): | ||
'''A tuple representing the full state of this object, used for | ||
hashing and equality testing.''' | ||
return tuple(value for key, value in self.__dict__.iteritems() | ||
if key not in (self._cache_irrelevant_attributes + | ||
['_cache_irrelevant_attributes'])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could potentially be refactored since it's the same code as before? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered it, but given that the function is almost trivial, I'm not sure it's worth introducing a common super-class for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I was thinking a sort of mixin class. |
||
|
||
unit = property(lambda self: get_unit(self.dim), | ||
doc='The `Unit` of this equation.') | ||
|
||
|
@@ -437,14 +449,6 @@ def __init__(self, type, varname, dimensions, var_type=FLOAT, expr=None, | |
if variable =='xi' or variable.startswith('xi_')]), | ||
doc='Stochastic variables in the RHS of this equation') | ||
|
||
|
||
_state_tuple = property(lambda self: (self.type, self.varname, | ||
self.dim, self.var_type, | ||
self.expr, tuple(self.flags)), | ||
doc='A tuple representing the full state of this ' | ||
'object, used for hashing and equality ' | ||
'testing.') | ||
|
||
def __eq__(self, other): | ||
if not isinstance(other, SingleEquation): | ||
return NotImplemented | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,5 +3,5 @@ numpy>=1.4.1 | |
pyparsing==1.5.7 | ||
sympy>=0.7.6 | ||
sphinxcontrib-issuetracker | ||
sphinx>=1.5 | ||
sphinx>=1.5,<1.6.3 | ||
ipython |
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.
This could be a class attribute maybe? Not sure if it's worth doing but maybe slightly more logical.
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.
Indeed! It would even simplify the code a bit, since it wouldn't appear in
__dict__
itself!