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

Implement support for subcoordinate systems in the y-axis #5840

Merged
merged 38 commits into from
Sep 26, 2023
Merged

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Aug 2, 2023

Implement support for subcoordinate systems in the y-axis

Screen Shot 2023-08-02 at 16 50 54

@droumis
Copy link
Member

droumis commented Aug 2, 2023

Just as a note-to-self to document where .subplot is in Bokeh:

https://github.com/bokeh/bokeh/blob/branch-3.3/src/bokeh/plotting/_figure.py#L225

@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2023

Codecov Report

Merging #5840 (23277bc) into main (4b6fb15) will decrease coverage by 65.10%.
Report is 1 commits behind head on main.
The diff coverage is 13.06%.

@@             Coverage Diff             @@
##             main    #5840       +/-   ##
===========================================
- Coverage   88.39%   23.30%   -65.10%     
===========================================
  Files         312      313        +1     
  Lines       64967    65149      +182     
===========================================
- Hits        57429    15180    -42249     
- Misses       7538    49969    +42431     
Flag Coverage Δ
ui-tests 23.30% <13.06%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
holoviews/plotting/bokeh/element.py 9.79% <5.26%> (-77.89%) ⬇️
holoviews/tests/plotting/bokeh/test_subcoordy.py 17.88% <17.88%> (ø)

... and 296 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@droumis
Copy link
Member

droumis commented Aug 3, 2023

This is great!

From a first pass, I've identified a few related issues:

  1. Different scaling across subplots: From my understanding, the subcoordinate_y parameter allows you to define a specific y-axis range for a subplot. The data that is plotted within each subplot will be scaled to fit within the y-axis range defined by subcoordinate_y, correct? This means that if the data in each subplot has a different min/max data amplitude (always the case), then each subplot will have its own y-axis scale compared to the other subplots, which is not desired. We want there to be a single scale bar applicable for a set of subplots.

  2. Floating y origin: The first value of each trace is floating within the subplot range, but the desired behavior would be for the first value to align with its ytick.

  3. RangeTool Y: Because each curve element is now on it's own subplot, and due to the issue of the RangeTool not being able to use an overlay for the target, it seems like the workaround of referencing a single element as the target no longer works to have the RangeTool control the y-axis linking. See video below; You can see the plot shows all the channels rather than a few, and that RangeTool works for the x axis but not the y.

Screen.Recording.2023-08-03.at.9.36.36.AM.mov
  1. y_range.bounds: The y_range.bounds applied to the overlay via backend_opts are no longer not respected; I can pan and zoom beyond them.

@droumis
Copy link
Member

droumis commented Aug 3, 2023

UPDATE: I am not so sure what's happening anymore.. the data for a particular curve seems to be allowed to go beyond the assigned subcoordinate_y range so I am just confused now.

image

Here is the code to reproduce:

import numpy as np
import pandas as pd
import holoviews as hv; hv.extension('bokeh')
from holoviews import opts
from holoviews import Dataset
from holoviews.plotting.links import RangeToolLink
from bokeh.models import HoverTool
import panel as pn; pn.extension()
from scipy.stats import zscore

n_channels = 10
n_seconds = 5
fs = 512
max_ch_disp = 5  # max channels to initially display
max_t_disp = 3 # max time in seconds to initially display

total_samples = n_seconds * fs
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = ['EEG {}'.format(i) for i in range(n_channels)]

channel_curves = []
ch_subcoord = 1./n_channels

hover = HoverTool(tooltips=[
    ("Channel", "@channel"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV")])

for i, channel_data in enumerate(data):
    ds = Dataset((time, channel_data, channels[i]), ["Time", "Amplitude", "channel"])
    channel_curves.append(hv.Curve(ds, "Time", ["Amplitude", "channel"], label=channels[i]).opts(
            subcoordinate_y=(i*ch_subcoord, (i+1)*ch_subcoord), color="black", line_width=1, 
            tools=[hover, 'xwheel_zoom'], shared_axes=False))

eeg_viewer = hv.Overlay(channel_curves, kdims="Channel").opts(
    padding=0, xlabel="Time (s)", ylabel="Channel",
    show_legend=False, aspect=1.5, responsive=True, # yticks=yticks,
    shared_axes=False, backend_opts={
        "x_range.bounds": (time.min(), time.max()),
        "y_range.bounds": (data.min(), data.max())})

yticks = [( (i*ch_subcoord + (i+1)*ch_subcoord) / 2, ich) for i, ich in enumerate(channels)]
y_positions, _ = zip(*yticks)

z_data = zscore(data, axis=1)

minimap = hv.Image((time, y_positions, z_data), ["Time (s)", "Channel"], "Amplitude (uV)")
minimap = minimap.opts(
    cmap="RdBu_r", colorbar=False, xlabel='', alpha=.5, yticks=[yticks[0], yticks[-1]],
    height=100, responsive=True, default_tools=[''], shared_axes=False, clim=(-z_data.std()*2.5, z_data.std()*2.5))


if len(channels) < max_ch_disp:
    max_ch_disp = len(channels)
max_y_disp = (max_ch_disp+2)*ch_subcoord

time_s = len(time)/fs
if time_s < max_t_disp:
    max_t_disp = time_s
    
RangeToolLink(minimap, list(eeg_viewer.values())[0], axes=["x", "y"],
              boundsx=(None, max_t_disp),
              boundsy=(None, max_y_disp))

eeg_app = pn.Column(pn.Row(eeg_viewer, min_height=500), minimap).servable(target='main')
eeg_app

@droumis
Copy link
Member

droumis commented Aug 3, 2023

pinging @mattpap so we can stay aligned on this effort

@droumis
Copy link
Member

droumis commented Aug 3, 2023

  1. VSpan: It seems like I cannot use hv.VSpan on an element or overlay of elements that use subcoordinate_y
import numpy as np
import holoviews as hv
hv.extension('bokeh')

x = np.linspace(0, 10, 100)
y = np.sin(x)
curve = hv.Curve((x, y)).opts(subcoordinate_y=(0,1))
vspan = hv.VSpan(2, 4)
curve *= vspan
curve
Traceback:
``` --------------------------------------------------------------------------- TypeError Traceback (most recent call last) File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/IPython/core/formatters.py:974, in MimeBundleFormatter.__call__(self, obj, include, exclude) 971 method = get_real_method(obj, self.print_method) 973 if method is not None: --> 974 return method(include=include, exclude=exclude) 975 return None 976 else:

File ~/src/holoviews/holoviews/core/dimension.py:1287, in Dimensioned.repr_mimebundle(self, include, exclude)
1280 def repr_mimebundle(self, include=None, exclude=None):
1281 """
1282 Resolves the class hierarchy for the class rendering the
1283 object using any display hooks registered on Store.display
1284 hooks. The output of all registered display_hooks is then
1285 combined and returned.
1286 """
-> 1287 return Store.render(self)

File ~/src/holoviews/holoviews/core/options.py:1423, in Store.render(cls, obj)
1421 data, metadata = {}, {}
1422 for hook in hooks:
-> 1423 ret = hook(obj)
1424 if ret is None:
1425 continue

File ~/src/holoviews/holoviews/ipython/display_hooks.py:280, in pprint_display(obj)
278 if not ip.display_formatter.formatters['text/plain'].pprint:
279 return None
--> 280 return display(obj, raw_output=True)

File ~/src/holoviews/holoviews/ipython/display_hooks.py:248, in display(obj, raw_output, **kwargs)
246 elif isinstance(obj, (CompositeOverlay, ViewableElement)):
247 with option_state(obj):
--> 248 output = element_display(obj)
249 elif isinstance(obj, (Layout, NdLayout, AdjointLayout)):
250 with option_state(obj):

File ~/src/holoviews/holoviews/ipython/display_hooks.py:142, in display_hook..wrapped(element)
140 try:
141 max_frames = OutputSettings.options['max_frames']
--> 142 mimebundle = fn(element, max_frames=max_frames)
143 if mimebundle is None:
144 return {}, {}

File ~/src/holoviews/holoviews/ipython/display_hooks.py:188, in element_display(element, max_frames)
185 if type(element) not in Store.registry[backend]:
186 return None
--> 188 return render(element)

File ~/src/holoviews/holoviews/ipython/display_hooks.py:69, in render(obj, **kwargs)
66 if renderer.fig == 'pdf':
67 renderer = renderer.instance(fig='png')
---> 69 return renderer.components(obj, **kwargs)

File ~/src/holoviews/holoviews/plotting/renderer.py:397, in Renderer.components(self, obj, fmt, comm, **kwargs)
394 embed = (not (dynamic or streams or self.widget_mode == 'live') or config.embed)
396 if embed or config.comms == 'default':
--> 397 return self._render_panel(plot, embed, comm)
398 return self._render_ipywidget(plot)

File ~/src/holoviews/holoviews/plotting/renderer.py:404, in Renderer._render_panel(self, plot, embed, comm)
402 doc = Document()
403 with config.set(embed=embed):
--> 404 model = plot.layout._render_model(doc, comm)
405 if embed:
406 return render_model(model, comm)

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/viewable.py:736, in Viewable._render_model(self, doc, comm)
734 if comm is None:
735 comm = state._comm_manager.get_server_comm()
--> 736 model = self.get_root(doc, comm)
738 if self._design and self._design.theme.bokeh_theme:
739 doc.theme = self._design.theme.bokeh_theme

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/layout/base.py:305, in Panel.get_root(self, doc, comm, preprocess)
301 def get_root(
302 self, doc: Optional[Document] = None, comm: Optional[Comm] = None,
303 preprocess: bool = True
304 ) -> Model:
--> 305 root = super().get_root(doc, comm, preprocess)
306 # ALERT: Find a better way to handle this
307 if hasattr(root, 'styles') and 'overflow-x' in root.styles:

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/viewable.py:658, in Renderable.get_root(self, doc, comm, preprocess)
656 wrapper = self._design._wrapper(self)
657 if wrapper is self:
--> 658 root = self._get_model(doc, comm=comm)
659 if preprocess:
660 self._preprocess(root)

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/layout/base.py:173, in Panel._get_model(self, doc, root, parent, comm)
171 root = root or model
172 self._models[root.ref['id']] = (model, parent)
--> 173 objects, _ = self._get_objects(model, [], doc, root, comm)
174 props = self._get_properties(doc)
175 props[self._property_mapping['objects']] = objects

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/layout/base.py:155, in Panel._get_objects(self, model, old_objects, doc, root, comm)
153 else:
154 try:
--> 155 child = pane._get_model(doc, root, model, comm)
156 except RerenderError as e:
157 if e.layout is not None and e.layout is not self:

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/pane/holoviews.py:411, in HoloViews._get_model(self, doc, root, parent, comm)
409 plot = self.object
410 else:
--> 411 plot = self._render(doc, comm, root)
413 plot.pane = self
414 backend = plot.renderer.backend

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/panel/pane/holoviews.py:506, in HoloViews._render(self, doc, comm, root)
503 if comm:
504 kwargs['comm'] = comm
--> 506 return renderer.get_plot(self.object, **kwargs)

File ~/src/holoviews/holoviews/plotting/bokeh/renderer.py:70, in BokehRenderer.get_plot(self_or_cls, obj, doc, renderer, **kwargs)
63 @bothmethod
64 def get_plot(self_or_cls, obj, doc=None, renderer=None, **kwargs):
65 """
66 Given a HoloViews Viewable return a corresponding plot instance.
67 Allows supplying a document attach the plot to, useful when
68 combining the bokeh model with another plot.
69 """
---> 70 plot = super().get_plot(obj, doc, renderer, **kwargs)
71 if plot.document is None:
72 plot.document = Document() if self_or_cls.notebook_context else curdoc()

File ~/src/holoviews/holoviews/plotting/renderer.py:241, in Renderer.get_plot(self_or_cls, obj, doc, renderer, comm, **kwargs)
238 defaults = [kd.default for kd in plot.dimensions]
239 init_key = tuple(v if d is None else d for v, d in
240 zip(plot.keys[0], defaults))
--> 241 plot.update(init_key)
242 else:
243 plot = obj

File ~/src/holoviews/holoviews/plotting/plot.py:943, in DimensionedPlot.update(self, key)
941 def update(self, key):
942 if len(self) == 1 and ((key == 0) or (key == self.keys[0])) and not self.drawn:
--> 943 return self.initialize_plot()
944 item = self.getitem(key)
945 self.traverse(lambda x: setattr(x, '_updated', True))

File ~/src/holoviews/holoviews/plotting/bokeh/element.py:2782, in OverlayPlot.initialize_plot(self, ranges, plot, plots)
2779 self.handles['plot'] = plot
2781 if plot and not self.overlaid:
-> 2782 self._update_plot(key, plot, element)
2783 self._update_ranges(element, ranges)
2785 panels = []

File ~/src/holoviews/holoviews/plotting/bokeh/element.py:877, in ElementPlot._update_plot(self, key, plot, element)
875 plot.update(**self._plot_properties(key, element))
876 if not self.multi_y:
--> 877 self._update_labels(key, plot, element)
878 self._update_title(key, plot, element)
879 self._update_grid(plot)

File ~/src/holoviews/holoviews/plotting/bokeh/element.py:885, in ElementPlot._update_labels(self, key, plot, element)
883 el = el[0] if el else element
884 dimensions = self._get_axis_dims(el)
--> 885 props = {axis: self._axis_properties(axis, key, plot, dim)
886 for axis, dim in zip(['x', 'y'], dimensions)}
887 xlabel, ylabel, zlabel = self._get_axis_labels(dimensions)
888 if self.invert_axes:

File ~/src/holoviews/holoviews/plotting/bokeh/element.py:885, in (.0)
883 el = el[0] if el else element
884 dimensions = self._get_axis_dims(el)
--> 885 props = {axis: self._axis_properties(axis, key, plot, dim)
886 for axis, dim in zip(['x', 'y'], dimensions)}
887 xlabel, ylabel, zlabel = self._get_axis_labels(dimensions)
888 if self.invert_axes:

File ~/src/holoviews/holoviews/plotting/bokeh/element.py:815, in ElementPlot._axis_properties(self, axis, key, plot, dimension, ax_mapping)
813 for i, (frame, coords) in enumerate(reversed(list(zip(self.current_frame, subcoords)))):
814 labels.append(frame.label or f'Trace {i}')
--> 815 ticks.append(np.mean(coords))
816 axis_props['ticker'] = FixedTicker(ticks=ticks)
817 if labels is not None:

File <array_function internals>:200, in mean(*args, **kwargs)

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/numpy/core/fromnumeric.py:3464, in mean(a, axis, dtype, out, keepdims, where)
3461 else:
3462 return mean(axis=axis, dtype=dtype, out=out, **kwargs)
-> 3464 return _methods._mean(a, axis=axis, dtype=dtype,
3465 out=out, **kwargs)

File ~/opt/miniconda3/envs/neuro-eeg-viewer/lib/python3.9/site-packages/numpy/core/_methods.py:194, in _mean(a, axis, dtype, out, keepdims, where)
192 ret = ret.dtype.type(ret / rcount)
193 else:
--> 194 ret = ret / rcount
196 return ret

TypeError: unsupported operand type(s) for /: 'NoneType' and 'int'
:Overlay
.Curve.I :Curve [x] (y)
.VSpan.I :VSpan [x,y]

</details>

@droumis
Copy link
Member

droumis commented Aug 4, 2023

@philippjfr's awesome latest pushes resolve a number of issues:

  1. Different scaling across subplots: The subcoordinate ranges are now computed at the OverlayPlot level
  2. The rangetoollink can now use an Overlay as a target. (fixes RangeToolLink doesn't work with overlays #4472)
  3. The workaround prior to # 2 was to link to a nested element.. but that was the cause of the first element being scaled in the subplot incorrectly.. so # 2 also fixed my observation above about the first curve going beyond the subplot y_range.
  4. VSpan: now works on elements with subcoordinate_y defined

Remaining issues:

  1. Floating y origin: The first value of each trace is floating within the subplot range, but the desired behavior would be for the first value to align with its ytick. UPDATE: dropped
  2. y_range.bounds: The y_range.bounds applied to the overlay via backend_opts are no longer respected; I can pan and zoom beyond them.

I think resolving Floating y origin: is the main thing left to address for this PR UPDATE: the main thing left is reasonable alignment of ytick with trace

Code:
import numpy as np
import pandas as pd
import holoviews as hv; hv.extension('bokeh')
from holoviews import opts
from holoviews import Dataset
from holoviews.plotting.links import RangeToolLink
from bokeh.models import HoverTool
import panel as pn; pn.extension()
from scipy.stats import zscore

n_channels = 10
n_seconds = 5
fs = 512
max_ch_disp = 5  # max channels to initially display
max_t_disp = 3 # max time in seconds to initially display

total_samples = n_seconds * fs
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = ['EEG {}'.format(i) for i in range(n_channels)]

channel_curves = []
ch_subcoord = 1./n_channels

hover = HoverTool(tooltips=[
    ("Channel", "@channel"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV")])

for i, channel_data in enumerate(data):
    ds = Dataset((time, channel_data, channels[i]), ["Time", "Amplitude", "channel"])
    channel_curves.append(hv.Curve(ds, "Time", ["Amplitude", "channel"], label=channels[i]).opts(
            subcoordinate_y=(i*ch_subcoord, (i+1)*ch_subcoord), color="black", line_width=1, 
            tools=[hover, 'xwheel_zoom'], shared_axes=False))

annotation = hv.VSpan(0.3, 0.5)
eeg_viewer = (hv.Overlay(channel_curves, kdims="Channel") * annotation).opts(
    padding=0, xlabel="Time (s)", ylabel="Channel",
    show_legend=False, aspect=1.5, min_height=500, responsive=True,
    shared_axes=False, backend_opts={
        "x_range.bounds": (time.min(), time.max()),
        "y_range.bounds": (0, 1)}) 

yticks = [( (i*ch_subcoord + (i+1)*ch_subcoord) / 2, ich) for i, ich in enumerate(channels)]
y_positions, _ = zip(*yticks)

z_data = zscore(data, axis=1)

minimap = hv.Image((time, y_positions, z_data), ["Time (s)", "Channel"], "Amplitude (uV)")
minimap = minimap.opts(
    cmap="RdBu_r", colorbar=False, xlabel='', alpha=.5, yticks=[yticks[0], yticks[-1]],
    height=100, responsive=True, default_tools=[''], shared_axes=False, clim=(-z_data.std()*2.5, z_data.std()*2.5))

if len(channels) < max_ch_disp:
    max_ch_disp = len(channels)
max_y_disp = (max_ch_disp+2)*ch_subcoord

time_s = len(time)/fs
if time_s < max_t_disp:
    max_t_disp = time_s
    
RangeToolLink(minimap, eeg_viewer, axes=["x", "y"],
              boundsx=(None, max_t_disp),
              boundsy=(None, max_y_disp))

eeg_app = pn.Column((eeg_viewer + minimap * annotation).cols(1), min_height=650).servable(target='main')
eeg_app

image

@jbednar
Copy link
Member

jbednar commented Aug 4, 2023

Very cool! I'm confused about the floating y origin; that already seems to be implemented in the plot above? In any case I don't think that is a must-have, but certainly a nice to have.

@droumis
Copy link
Member

droumis commented Aug 7, 2023

Very cool! I'm confused about the floating y origin; that already seems to be implemented in the plot above? In any case I don't think that is a must-have, but certainly a nice to have.

I think it just appears to be aligned with the y axis when the individual traces are scaled down a lot. Here is the same code with some real data.. it's scaled down so much that it isn't that useful of a view:

(Relevant line per i channel: subcoordinate_y=(i*(1./n_channels), (i+1)*(1./n_channels)))
image

However, when I 'scale up' the traces by adjusting the range that each subplot is allowed to be within, the data no longer aligns with the y-tick, and it's especially bad when panning across the channels.

(Relevant line per i channel: subcoordinate_y=(i*(1./n_channels), (i+10)*(1./n_channels)))

Screen.Recording.2023-08-07.at.8.30.48.AM.mov

I think instead of defining the subplot by the upper and lower y_range that it's allowed to be in, we will need to define it by the offset for the y-origin, and then have a separate parameter for the scaling to use for each group of curves. @philippjfr, @mattpap do you think this would be possible without major changes to Bokeh?

Here is a demonstration of overlap that we want to achieve. We also want to be able to expose the channel-level scaling/zooming which would cause more or less overlap across channels.. which reinforces the need for an origin offset approach rather than an upper and lower y_range approach.
image

@mattpap
Copy link

mattpap commented Aug 11, 2023

However, when I 'scale up' the traces by adjusting the range that each subplot is allowed to be within, the data no longer aligns with the y-tick, and it's especially bad when panning across the channels.

If I understand the issue correctly, then the solution is to compute y source range for each sub-coordinate based on the data indices in the viewport (i.e. indices contained within the cartesian frame) and not all the available indices as it is done now. That would mean using data ranges for y source ranges and changes to bokeh, to allow data ranges optionally to consider the viewport (so called masked indices). I doubt this can be hacked around in Holoviews, because masked indices are not exposed to Python APIs in bokeh.

@droumis
Copy link
Member

droumis commented Aug 15, 2023

However, when I 'scale up' the traces by adjusting the range that each subplot is allowed to be within, the data no longer aligns with the y-tick, and it's especially bad when panning across the channels.

If I understand the issue correctly, then the solution is to compute y source range for each sub-coordinate based on the data indices in the viewport (i.e. indices contained within the cartesian frame) and not all the available indices as it is done now. That would mean using data ranges for y source ranges and changes to bokeh, to allow data ranges optionally to consider the viewport (so called masked indices). I doubt this can be hacked around in Holoviews, because masked indices are not exposed to Python APIs in bokeh.

My interpretation of what you are saying is that we would need to dynamically adjust the y-range for each trace based on what's currently visible, rather than using a fixed y-range for all available data. I think this means that each y-tick would then be centered on the y-range of each subplot's entire viewport-visible data, correct? Or would the y-tick indeed align with the first (left-most) data sample in the current viewport?

@philippjfr
Copy link
Member Author

I think what @mattpap is suggesting may indeed be a nice improvement but I'm not sure it addresses the actual request that @droumis had, which I think merely requires setting the target ranges in such a way that the beginning of the trace aligns with the tick and has the desired level of scaling so the individual traces are visible.

@droumis
Copy link
Member

droumis commented Aug 17, 2023

@philippjfr, @mattpap. To simplify things, I've decided to now drop the requested alignment of the y-tick with the left-most data point. The type of data we are working with for CZI should have very low frequencies ('slow drift') removed prior to plotting, so I think it's good enough as long as the y-tick is reasonably close to the trace. However, reasonable alignment of the ytick when panning with overlapping ranges is still an issue with this PR, as was shown in this video:

Screen.Recording.2023-08-07.at.8.30.48.AM.mov

Any idea how to resolve this, @philippjfr? As @mattpap mentioned in the meeting yesterday, he does not see such behavior in a pure Bokeh implementation.

If we can get this resolved (and also have zoom tools to scale subcoordinate ranges), then I think that's all we need for now.

@droumis
Copy link
Member

droumis commented Aug 29, 2023

TODOS:

  • Fix the alignment with yticks when ranges overlap, especially during panning (see previous post). Try first by making the axes categorical, since the pure Bokeh version doesn't have this problem and the difference in implementations seems to be that the Bokeh version uses a FactorRange as the target range.
  • Pull out the fix for allowing links at overlay level into separate PR
  • Finish and merge zooming of subplots merged in Bokeh
  • Define a more automated API in HoloViews so users aren't required to explicitly specify subcoordinate_y ranges. Instead of individual subcoordinate_y ranges at the Curve element level, I think a better approach would be to have subcoordinate_y at the Container element level (e.g. NdOverlay/Overlay) and a value of 'True' would infer the range mapping based on the number of subplots. Users could also control the initial amount of overlay between the subplots by specifying a percentage (0-1), effectively allowing them to control y_ranges but without asking users to explicitly think about the target range mapping. Here is some pseudocode for how think the API should look:
channel_curves = {}
for channel_data in ds:
    channel_curves[channel_data.channel] = hv.Curve(channel_data.data, "Time", ["Amplitude", "Channel"])

plot = hv.NdOverlay(channel_curves, kdims="Channel", subcoordinate_y=True)

# Alternatively, to get 10% overlap:

plot = hv.NdOverlay(channel_curves, kdims="Channel", subcoordinate_y=0.1)

thoughts?

@mattpap
Copy link

mattpap commented Aug 30, 2023

Reproduction in pure bokeh for both categorical and linear axis approaches (change categorical global variable):

import numpy as np

from bokeh.core.properties import field
from bokeh.io import show
from bokeh.models import (ColumnDataSource, DataRange1d, FactorRange, HoverTool,
                          Range1d, WheelZoomTool, ZoomInTool, ZoomOutTool)
from bokeh.palettes import Category10
from bokeh.plotting import figure

np.random.seed(0)

categorical = False

n_channels = 10
n_seconds = 15
fs = 512
max_ch_disp = 5  # max channels to initially display
max_t_disp = 3 # max time in seconds to initially display

total_samples = n_seconds * fs
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = [f'EEG {i}' for i in range(n_channels)]

hover = HoverTool(tooltips=[
    ("Channel", "$name"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV"),
])

x_range = Range1d(start=time.min(), end=time.max())
#x_range = DataRange1d()
#y_range = DataRange1d() # bug
if categorical:
    y_range = FactorRange(factors=channels)
else:
    y_range = Range1d(start=-0.5, end=len(channels) - 1 + 0.5)

p = figure(x_range=x_range, y_range=y_range, lod_threshold=None)

source = ColumnDataSource(data=dict(time=time))
renderers = []

for i, channel in enumerate(channels):
    if categorical:
        y_target=Range1d(start=i, end=i + 1)
    else:
        y_target=Range1d(start=i - 0.5, end=i + 0.5)

    xy = p.subplot(
        x_source=p.x_range,
        y_source=Range1d(start=data[i].min(), end=data[i].max()),
        #y_source=DataRange1d(),
        x_target=p.x_range,
        y_target=y_target,
    )

    source.data[channel] = data[i]
    line = xy.line(field("time"), field(channel), color=Category10[10][i], source=source, name=channel)
    renderers.append(line)

if not categorical:
    from bokeh.models import FixedTicker
    ticks = list(range(len(channels)))
    p.yaxis.ticker = FixedTicker(ticks=ticks)
    p.yaxis.major_label_overrides = {i: f"EEG {i}" for i in ticks}

wheel_zoom = WheelZoomTool()#renderers=renderers)
zoom_in = ZoomInTool(renderers=renderers)
zoom_out = ZoomOutTool(renderers=renderers)

p.add_tools(wheel_zoom, zoom_in, zoom_out, hover)
p.toolbar.active_scroll = wheel_zoom

show(p)

@philippjfr
Copy link
Member Author

Interesting, so in your pure-bokeh version I can't reproduce the panning issue.

@philippjfr
Copy link
Member Author

I found the source of the problem, I had been adding the target ranges to the extra_y_ranges dictionary to make it easier to pass them down to the subplots. If I pop them before plotting the problem goes away.

@maximlt
Copy link
Member

maximlt commented Sep 6, 2023

Just for reference some pure Bokeh code to reproduce what we want i.e. a plot with traces on subcoordinates with its minimap.

Kapture 2023-09-06 at 17 07 12

Code
import numpy as np

from bokeh.layouts import column
from bokeh.core.properties import field
from bokeh.io import show
from bokeh.models import (ColumnDataSource, DataRange1d, FactorRange, HoverTool,
                          Range1d, WheelZoomTool, ZoomInTool, ZoomOutTool, RangeTool)
from bokeh.palettes import Category10
from bokeh.plotting import figure

from scipy.stats import zscore

np.random.seed(0)

categorical = False

n_channels = 10
n_seconds = 15
fs = 512
max_ch_disp = 5  # max channels to initially display
max_t_disp = 3 # max time in seconds to initially display

total_samples = n_seconds * fs
time = np.linspace(0, n_seconds, total_samples)
data = np.random.randn(n_channels, total_samples).cumsum(axis=1)
channels = [f'EEG {i}' for i in range(n_channels)]

hover = HoverTool(tooltips=[
    ("Channel", "$name"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV"),
])

x_range = Range1d(start=time.min(), end=time.max())
#x_range = DataRange1d()
#y_range = DataRange1d() # bug
if categorical:
    y_range = FactorRange(factors=channels)
else:
    y_range = Range1d(start=-0.5, end=len(channels) - 1 + 0.5)

p = figure(x_range=x_range, y_range=y_range, lod_threshold=None)

source = ColumnDataSource(data=dict(time=time))
renderers = []

for i, channel in enumerate(channels):
    if categorical:
        y_target=Range1d(start=i, end=i + 1)
    else:
        y_target=Range1d(start=i - 0.5, end=i + 0.5)

    xy = p.subplot(
        x_source=p.x_range,
        y_source=Range1d(start=data[i].min(), end=data[i].max()),
        #y_source=DataRange1d(),
        x_target=p.x_range,
        y_target=y_target,
    )

    source.data[channel] = data[i]
    line = xy.line(field("time"), field(channel), color=Category10[10][i], source=source, name=channel)
    renderers.append(line)

if not categorical:
    from bokeh.models import FixedTicker
    ticks = list(range(len(channels)))
    p.yaxis.ticker = FixedTicker(ticks=ticks)
    p.yaxis.major_label_overrides = {i: f"EEG {i}" for i in ticks}

wheel_zoom = WheelZoomTool()#renderers=renderers)
zoom_in = ZoomInTool(renderers=renderers)
zoom_out = ZoomOutTool(renderers=renderers)

p.add_tools(wheel_zoom, zoom_in, zoom_out, hover)
p.toolbar.active_scroll = wheel_zoom

z_data = zscore(data, axis=1)

range_tool = RangeTool(x_range=p.x_range, y_range=p.y_range)
range_tool.x_range.update(start=4, end=6)
range_tool.y_range.update(start=2, end=8)

select = figure(height=130, #y_range=p.y_range,
                tools="", toolbar_location=None)
select.image(image=[z_data], x=0, y=0, dw=15, dh=11, palette="Sunset11")
select.add_tools(range_tool)

show(column(p, select))

@maximlt
Copy link
Member

maximlt commented Sep 22, 2023

I have made changes based on your comments @hoxbro , thanks for the thorough review!

On the edge cases you shared, the error displayed in the first one is indeed a little terse 🙃 although the error is pretty obvious in your example. After your discussion with Philipp, let me know if you want me to investigate how to raise a more informative error (I guess I'll have to traverse all the elements to check they have unique labels, maybe there's an API for that?). The "same subcordinate_y case" is a feature request to me, please somehow explode the ticks when they overlap. The last case with "only some curves have enabled subcoordinate_y" will hopefully be supported soon. I'm thinking that users could provide a range for each element that doesn't have to be between 0 and 1, we'd traverse all the elements and collect these ranges to compute v0 and v1.

On your comment on whether multi axes are subcoordinates are supported together, I believe that is not the case (haven't tried though!) and that it's very much on the edge.

@maximlt
Copy link
Member

maximlt commented Sep 25, 2023

@hoxbro @philippjfr is there any remaining changes required for this PR to be merged?

@hoxbro
Copy link
Member

hoxbro commented Sep 25, 2023

Two things for me:

Is it too tedious to add a way to detect if the labels are the same? If it is please open a small issue so we can keep track of it.

The other thing is the twin axis definitely looks wrong:

import holoviews as hv
import numpy as np

hv.extension("bokeh")

x = np.linspace(0, 10*np.pi)

curves = [
    hv.Curve((x + i*np.pi/2, np.sin(x)), label=f'Label {i}').opts(subcoordinate_y=True, subcoordinate_scale=1.2)
    for i in range(3)
]

curves2 = [
    hv.Curve((x + i*np.pi/2, np.sin(x)), label=f'Label {i}', vdims=["twin"]).opts(subcoordinate_y=True, subcoordinate_scale=1.2)
    for i in range(3, 6)
]

hv.Overlay(curves + curves2).opts(show_legend=False, multi_y=True)

Outputs (no labels, and only show the first curve)
image

Zooming in on the y-axis, and then zoom on the plot you get:
image

Can we make a check in _subcoord_overlaid that multi_y and raise an error if both are enabled?

@maximlt
Copy link
Member

maximlt commented Sep 25, 2023

Is it too tedious to add a way to detect if the labels are the same?

Not sure it's tedious, I just haven't been able to find the internal API to do it, I'll look for it but welcome any pointer! I hope having to traverse the objects won't be too costly, since an aspect of this review was performance optimizations.

Can we make a check in _subcoord_overlaid that multi_y and raise an error if both are enabled?

I'll add a check. Not sure the property is the right place to make that check though, I guess those kind of checks should all be made in the same place, at some point in the initialization phase, I just have to find where this is.

@droumis
Copy link
Member

droumis commented Sep 26, 2023

Found an issue. When trying to overlay an annotation (hv.VSpan), I get ValueError: Every element wrapped in a subcoordinate_y overlay must have a label.

@droumis droumis self-requested a review September 26, 2023 11:43
@droumis
Copy link
Member

droumis commented Sep 26, 2023

Also, running rasterize on the minimap breaks the RangeToolLink. There is no error, but the RangeTool Box no longer appears. Maybe something that rasterize does changes the axis in some way that is not compatible with subcoordinates?

Code
import numpy as np
import holoviews as hv
from bokeh.models import HoverTool
from holoviews.plotting.links import RangeToolLink
from scipy.stats import zscore
from holoviews.operation.datashader import rasterize

hv.extension('bokeh')

N_CHANNELS = 10
N_SECONDS = 5
SAMPLING_RATE = 200
INIT_FREQ = 2  # Initial frequency in Hz
FREQ_INC = 5  # Frequency increment
AMPLITUDE = 1

# Generate time and channel labels
total_samples = N_SECONDS * SAMPLING_RATE
time = np.linspace(0, N_SECONDS, total_samples)
channels = [f'EEG {i}' for i in range(N_CHANNELS)]

# Generate sine wave data
data = np.array([AMPLITUDE * np.sin(2 * np.pi * (INIT_FREQ + i * FREQ_INC) * time)
                     for i in range(N_CHANNELS)])

hover = HoverTool(tooltips=[
    ("Channel", "@channel"),
    ("Time", "$x s"),
    ("Amplitude", "$y µV")
])

channel_curves = []
for channel, channel_data in zip(channels, data):
    ds = hv.Dataset((time, channel_data, channel), ["Time", "Amplitude", "channel"])
    curve = hv.Curve(ds, "Time", ["Amplitude", "channel"], label=channel)
    curve.opts(
        subcoordinate_y=True, color="black", line_width=1, tools=[hover],
    )
    channel_curves.append(curve)

eeg = hv.Overlay(channel_curves, kdims="Channel").opts(
    xlabel="Time (s)", ylabel="Channel", show_legend=False, aspect=3, responsive=True,
)

y_positions = range(N_CHANNELS)
yticks = [(i , ich) for i, ich in enumerate(channels)]

z_data = zscore(data, axis=1)

minimap = rasterize(hv.Image((time, y_positions , z_data), ["Time (s)", "Channel"], "Amplitude (uV)"))
minimap = minimap.opts(
    cmap="RdBu_r", xlabel='Time (s)', alpha=.5, yticks=[yticks[0], yticks[-1]],
    height=150, responsive=True, default_tools=[], clim=(-z_data.std(), z_data.std())
)

RangeToolLink(
    minimap, eeg, axes=["x", "y"],
    boundsx=(None, 2), boundsy=(None, 6.5)
)

dashboard = (eeg + minimap).opts(merge_tools=False).cols(1)
dashboard

image

@droumis
Copy link
Member

droumis commented Sep 26, 2023

Linking with rasterize on the minimap seems to work using the non-subcoordinate_y approach (see code) and the current HoloViews main branch, meaning that the link to overlay PR is probably not at fault.

CODE: offset approach with RangeToolLink to an overlay
import numpy as np
import holoviews as hv
from bokeh.models import HoverTool
from holoviews.plotting.links import RangeToolLink
from scipy.stats import zscore
from holoviews.operation.datashader import rasterize

hv.extension('bokeh')


N_CHANNELS = 10
N_SECONDS = 5
SAMPLING_RATE = 200
INIT_FREQ = 2  # Initial frequency in Hz
FREQ_INC = 5  # Frequency increment
AMPLITUDE = 1

# Generate time and channel labels
total_samples = N_SECONDS * SAMPLING_RATE
time = np.linspace(0, N_SECONDS, total_samples)
channels = [f'EEG {i}' for i in range(N_CHANNELS)]

# Generate sine wave data
data = np.array([AMPLITUDE * np.sin(2 * np.pi * (INIT_FREQ + i * FREQ_INC) * time)
                     for i in range(N_CHANNELS)])

max_ch_disp = 5  # max channels to initially display
max_t_disp = 2 # max time in seconds to initially display
spacing = 3.5  # Spacing between channels
clim_mul = 1.2 # color range multiplier for minimap. lower will saturate more.

# Annotations
annotation = hv.VSpan(1, 1.5) # example annotation (start, end) time

# Create a hv.Curve element per chan
hover = HoverTool(tooltips=[
    ("Channel", "@channel"),
    ("Time", "$x s"),
    ("Amplitude", "@original_amplitude µV")])

# set offset for data (non-subcoord approach)
offset = np.std(data) * spacing
offset_data = data + (np.arange(len(data))[:, np.newaxis] * offset)
max_data = offset_data.max()

y_positions = np.arange(len(channels)) * offset
yticks = list(zip(y_positions, channels))

channel_curves = []
for i, channel_data in enumerate(data):
    ds = hv.Dataset((time, offset_data[i,:], channel_data, channels[i]), 
                 ["Time", "Amplitude", "original_amplitude", "channel"])
    channel_curves.append(
        hv.Curve(ds, "Time", ["Amplitude", "original_amplitude", "channel"]).opts(
            color="black", line_width=1,
            tools=[hover, 'xwheel_zoom'], shared_axes=False))
        
eeg_viewer = (annotation * hv.Overlay(channel_curves, kdims="Channel")).opts(
    padding=0, xlabel="Time (s)", ylabel="Channel",
    yticks=yticks, show_legend=False, aspect=3, responsive=True,
    shared_axes=False, backend_opts={
        "x_range.bounds": (time.min(), time.max()),
        "y_range.bounds": (data.min(), max_data)})

z_data = zscore(data, axis=1)
minimap = rasterize(hv.Image((time, y_positions, z_data), ["Time (s)", "Channel"], "Amplitude (uV)"))
minimap = minimap.opts(
    cmap="RdBu_r", colorbar=False, xlabel='', alpha=.8, yticks=[yticks[0], yticks[-1]],
    height=100, responsive=True, default_tools=[''], shared_axes=False,
    clim=(-z_data.std()*clim_mul, z_data.std()*clim_mul))

RangeToolLink(minimap, eeg_viewer, axes=["x", "y"],
              boundsx=(None, max_t_disp),
              boundsy=(None, max_y_disp))

eeg_app = (eeg_viewer + minimap * annotation).cols(1)
eeg_app

image

@maximlt
Copy link
Member

maximlt commented Sep 26, 2023

@droumis Linking a rasterized image to an overlay works with this hack from of the first commits of this PR:
image

That hack has been replaced in #5881, I was trying to minimize the impact of propagating plot_id, apparently that broke this specific use case.

@maximlt maximlt merged commit 38e0b39 into main Sep 26, 2023
11 checks passed
@maximlt maximlt deleted the subcoordinate branch September 26, 2023 16:01
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants