-
-
Notifications
You must be signed in to change notification settings - Fork 402
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 memoization to dynamic Callable class #1063
Changes from 3 commits
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 |
---|---|---|
|
@@ -404,7 +404,10 @@ class Callable(param.Parameterized): | |
allowing their inputs (and in future outputs) to be defined. | ||
This makes it possible to wrap DynamicMaps with streams and | ||
makes it possible to traverse the graph of operations applied | ||
to a DynamicMap. | ||
to a DynamicMap. Additionally a Callable will memoize the last | ||
returned value based on the arguments to the function and the | ||
state of all streams on its inputs, avoiding calling the function | ||
unncessarily. | ||
""" | ||
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. Typo. |
||
|
||
callable_function = param.Callable(default=lambda x: x, doc=""" | ||
|
@@ -413,8 +416,22 @@ class Callable(param.Parameterized): | |
inputs = param.List(default=[], doc=""" | ||
The list of inputs the callable function is wrapping.""") | ||
|
||
def __init__(self, **params): | ||
super(Callable, self).__init__(**params) | ||
self._memoized = {} | ||
|
||
def __call__(self, *args, **kwargs): | ||
return self.callable_function(*args, **kwargs) | ||
inputs = [inp for inp in self.inputs if isinstance(inp, DynamicMap)] | ||
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. Very minor point - I would call the loop variable |
||
streams = [s for inp in inputs for s in get_nested_streams(inp)] | ||
values = tuple(tuple(sorted(s.contents.items())) for s in streams) | ||
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 means it is memoizing on the stream parameters in ascending alphanumeric order. I think that is fine (and makes sense!) but it is worth noting that this is one particular policy for how streams parameters are ordered into a tuple key. This is important to stay consistent if we ever need it somewhere else and I am now wondering if it might be worth having a utility to codify the idea....up to you. 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 really doesn't matter much, it just needs to be consistently ordered. If we're using it somewhere else it can use the same scheme or another scheme without having any effect here. 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. True - my point is that we should try to be consistent anyway! |
||
key = args + tuple(sorted(kwargs.items())) + values | ||
|
||
if key in self._memoized: | ||
return self._memoized[key] | ||
else: | ||
ret = self.callable_function(*args, **kwargs) | ||
self._memoized = {key : ret} | ||
return ret | ||
|
||
|
||
def get_nested_streams(dmap): | ||
|
@@ -500,6 +517,8 @@ class DynamicMap(HoloMap): | |
""") | ||
|
||
def __init__(self, callback, initial_items=None, **params): | ||
if not isinstance(callback, Callable) and not isinstance(callback, types.GeneratorType): | ||
callback = Callable(callable_function=callback) | ||
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 means all callbacks will now be wrapped in 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. For instance, we might want an example of the user declaring a |
||
super(DynamicMap, self).__init__(initial_items, callback=callback, **params) | ||
|
||
# Set source to self if not already specified | ||
|
@@ -514,7 +533,6 @@ def __init__(self, callback, initial_items=None, **params): | |
|
||
self.call_mode = self._validate_mode() | ||
self.mode = 'bounded' if self.call_mode == 'key' else 'open' | ||
self._dimensionless_cache = False | ||
|
||
|
||
def _initial_key(self): | ||
|
@@ -762,7 +780,7 @@ def __getitem__(self, key): | |
try: | ||
dimensionless = util.dimensionless_contents(get_nested_streams(self), | ||
self.kdims, no_duplicates=False) | ||
if (dimensionless and not self._dimensionless_cache): | ||
if dimensionless: | ||
raise KeyError('Using dimensionless streams disables DynamicMap cache') | ||
cache = super(DynamicMap,self).__getitem__(key) | ||
# Return selected cache items in a new DynamicMap | ||
|
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.
I think either 'avoiding calls to the function' or 'to avoid calling the function ...' would be read better.