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

Various fixes for bokeh static_source optimization #1413

Merged
merged 5 commits into from
May 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 2 deletions holoviews/core/dimension.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
from ..core.util import (basestring, sanitize_identifier,
group_sanitizer, label_sanitizer, max_range,
find_range, dimension_sanitizer, OrderedDict,
bytes_to_unicode, unicode, dt64_to_dt, unique_array)
bytes_to_unicode, unicode, dt64_to_dt, unique_array,
builtins)
from .options import Store, StoreOptions
from .pprint import PrettyPrinter

Expand Down Expand Up @@ -479,7 +480,7 @@ class LabelledData(param.Parameterized):

_deep_indexable = False

def __init__(self, data, id=None, **params):
def __init__(self, data, id=None, plot_id=None, **params):
"""
All LabelledData subclasses must supply data to the
constructor, which will be held on the .data attribute.
Expand All @@ -488,6 +489,7 @@ def __init__(self, data, id=None, **params):
"""
self.data = data
self.id = id
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we ever actually set the id via the constructor?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, probably for clone. Nevermind my question!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, clone does all the time.

self._plot_id = plot_id or builtins.id(self)
if isinstance(params.get('label',None), tuple):
(alias, long_name) = params['label']
label_sanitizer.add_aliases(**{alias:long_name})
Expand Down Expand Up @@ -531,6 +533,7 @@ def clone(self, data=None, shared_data=True, new_type=None, *args, **overrides):

if data is None and shared_data:
data = self.data
settings['plot_id'] = self._plot_id
# Apply name mangling for __ attribute
pos_args = getattr(self, '_' + type(self).__name__ + '__pos_params', [])
return clone_type(data, *args, **{k:v for k,v in settings.items()
Expand Down
2 changes: 1 addition & 1 deletion holoviews/core/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def __init__(self, items=None, identifier=None, parent=None, **kwargs):
self.__dict__['_max_cols'] = 4
if items and all(isinstance(item, Dimensioned) for item in items):
items = self._process_items(items)
params = {p: kwargs.pop(p) for p in list(self.params().keys())+['id'] if p in kwargs}
params = {p: kwargs.pop(p) for p in list(self.params().keys())+['id', 'plot_id'] if p in kwargs}
AttrTree.__init__(self, items, identifier, parent, **kwargs)
Dimensioned.__init__(self, self.data, **params)

Expand Down
1 change: 1 addition & 0 deletions holoviews/core/ndmapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ def clone(self, data=None, shared_data=True, new_type=None, *args, **overrides):

if data is None and shared_data:
data = self.data
settings['plot_id'] = self._plot_id
# Apply name mangling for __ attribute
pos_args = getattr(self, '_' + type(self).__name__ + '__pos_params', [])
with item_check(not shared_data and self._check_items):
Expand Down
1 change: 1 addition & 0 deletions holoviews/core/spaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,7 @@ def clone(self, data=None, shared_data=True, new_type=None, link_inputs=True,
"""
if data is None and shared_data:
data = self.data
overrides['plot_id'] = self._plot_id
clone = super(UniformNdMapping, self).clone(overrides.pop('callback', self.callback),
shared_data, new_type,
*(data,) + args, **overrides)
Expand Down
5 changes: 5 additions & 0 deletions holoviews/core/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
except:
from collections import OrderedDict

try:
import __builtin__ as builtins
except:
import builtins as builtins

datetime_types = (np.datetime64, dt.datetime)

try:
Expand Down
2 changes: 2 additions & 0 deletions holoviews/element/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ def clone(self, *args, **overrides):
containing the specified args and kwargs.
"""
settings = dict(self.get_param_values(), **overrides)
if not args:
settings['plot_id'] = self._plot_id
return self.__class__(*args, **settings)


Expand Down
10 changes: 7 additions & 3 deletions holoviews/plotting/bokeh/element.py
Original file line number Diff line number Diff line change
Expand Up @@ -727,11 +727,14 @@ def initialize_plot(self, ranges=None, plot=None, plots=None, source=None):
# Get data and initialize data source
empty = False
if self.batched:
current_id = tuple(element.traverse(lambda x: x._plot_id, [Element]))
data, mapping = self.get_batched_data(element, ranges, empty)
else:
data, mapping = self.get_data(element, ranges, empty)
current_id = element._plot_id
if source is None:
source = self._init_datasource(data)
self.handles['previous_id'] = current_id
self.handles['source'] = source

properties = self._glyph_properties(plot, style_element, source, ranges)
Expand Down Expand Up @@ -804,15 +807,16 @@ def update_frame(self, key, ranges=None, plot=None, element=None, empty=False):
# Cache frame object id to skip updating data if unchanged
previous_id = self.handles.get('previous_id', None)
if self.batched:
current_id = sum(element.traverse(lambda x: id(x.data), [Element]))
current_id = tuple(element.traverse(lambda x: x._plot_id, [Element]))
else:
current_id = id(element.data)
current_id = element._plot_id
self.handles['previous_id'] = current_id
self.static_source = self.dynamic and (current_id == previous_id)
self.static_source = (self.dynamic and (current_id == previous_id))
if self.batched:
data, mapping = self.get_batched_data(element, ranges, empty)
else:
data, mapping = self.get_data(element, ranges, empty)

if not self.static_source:
self._update_datasource(source, data)

Expand Down
2 changes: 1 addition & 1 deletion holoviews/plotting/bokeh/plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def sync_sources(self):
from the same object.
"""
get_sources = lambda x: (id(x.current_frame.data), x)
filter_fn = lambda x: (x.shared_datasource and x.current_frame and
filter_fn = lambda x: (x.shared_datasource and x.current_frame is not None and
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm: is this a a fix unrelated to the optimization itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems so, not sure why I pushed it here, but it is an important fix.

not isinstance(x.current_frame.data, np.ndarray)
and 'source' in x.handles)
data_sources = self.traverse(get_sources, [filter_fn])
Expand Down
7 changes: 4 additions & 3 deletions holoviews/plotting/bokeh/tabular.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,12 @@ def current_handles(self):
if self.static and not self.dynamic:
return handles


element = self.current_frame
previous_id = self.handles.get('previous_id', None)
current_id = id(self.current_frame.data) if self.current_frame else None
current_id = None if self.current_frame is None else element._plot_id
for handle in self._update_handles:
if (handle == 'source' and self.dynamic and
current_id == previous_id):
if (handle == 'source' and self.dynamic and current_id == previous_id):
continue
if handle in self.handles:
handles.append(self.handles[handle])
Expand Down
38 changes: 37 additions & 1 deletion tests/testplotinstantiation.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
Scatter3D, Path, Polygons, Bars, Text,
BoxWhisker, HLine)
from holoviews.element.comparison import ComparisonTestCase
from holoviews.streams import PointerXY, PointerX
from holoviews.streams import Stream, PointerXY, PointerX
from holoviews.operation import gridmatrix
from holoviews.plotting import comms
from holoviews.plotting.util import rgb2hex
Expand All @@ -46,6 +46,7 @@
from holoviews.plotting.bokeh.util import bokeh_version
bokeh_renderer = Store.renderers['bokeh']
from holoviews.plotting.bokeh.callbacks import Callback, PointerXCallback
from bokeh.document import Document
from bokeh.models import (
Div, ColumnDataSource, FactorRange, Range1d, Row, Column,
ToolbarBox, FixedTicker, FuncTickFormatter
Expand Down Expand Up @@ -382,6 +383,41 @@ def test_layout_update_visible(self):
self.assertFalse(subplot1.handles['glyph_renderer'].visible)
self.assertTrue(subplot2.handles['glyph_renderer'].visible)

def test_static_source_optimization_not_skipping_new_element(self):
global data
data = np.ones((5, 5))
def get_img(test):
global data
data *= test
return Image(data)
stream = Stream.define(str('Test'), test=1)()
dmap = DynamicMap(get_img, streams=[stream])
plot = bokeh_renderer.get_plot(dmap, doc=Document())
source = plot.handles['source']
self.assertEqual(source.data['image'][0].mean(), 1)
stream.event(test=2)
self.assertFalse(plot.static_source)
self.assertEqual(source.data['image'][0].mean(), 2)
self.assertIn(source, plot.current_handles)

def test_static_source_optimization(self):
global data
data = np.ones((5, 5))
img = Image(data)
def get_img(test):
global data
data *= test
return img
stream = Stream.define(str('Test'), test=1)()
dmap = DynamicMap(get_img, streams=[stream])
plot = bokeh_renderer.get_plot(dmap, doc=Document())
source = plot.handles['source']
self.assertEqual(source.data['image'][0].mean(), 1)
stream.event(test=2)
self.assertTrue(plot.static_source)
self.assertEqual(source.data['image'][0].mean(), 2)
self.assertNotIn(source, plot.current_handles)

def test_batched_plot(self):
overlay = NdOverlay({i: Points(np.arange(i)) for i in range(1, 100)})
plot = bokeh_renderer.get_plot(overlay)
Expand Down