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

[BUG] CDS updates on figure data that change the axis magnitude will trigger a recomputation of the complete layout #12129

Open
muendlein opened this issue May 14, 2022 · 4 comments

Comments

@muendlein
Copy link
Contributor

Software versions

Python version : 3.7.3 (v3.7.3:ef4ec6ed12, Mar 25 2019, 22:22:05) [MSC v.1916 64 bit (AMD64)]
IPython version : (not installed)
Tornado version : 6.1
Bokeh version : 3.0.0dev2

Browser name and version

No response

Jupyter notebook / Jupyter Lab version

No response

Expected behavior

Data updates on plots via CDS should not unnecessarily invalidate the complete layout but instead only adjust the figure that is updated.

Observed behavior

Data updates that cause the magnitude of the y-axis to change (i.e. from single digits to double digits or the other way round) will invalidate the complete layout and repaint it from scratch again. This seems rather unnecessary and is a huge performance bottleneck.

This can be reproduced with my example below: here we have 50 figures and tables respectively with only the first plot table combination being updated via the buttons on the left. Both buttons will trigger random data updates via CDS but scale differently (< 10 and < 100). Everytime magnitudes change the repainting of the layout is triggered which takes around 1 seconds to complete (see screenshot below, updates without magnitude changes work nearly instantly). If instead all 50 plots are updated this behaviour scales linearly (50 seconds to update) which is rather undesired.

As a side note: embedding the layout within a tab widget makes things even worse as the performance is decreased by a factor of 2.

Example code

import numpy as np
import pandas as pd

from bokeh.io import output_notebook, curdoc

from bokeh.layouts import row, layout, column
from bokeh.layouts import grid
from bokeh.models import CustomJS, ColumnDataSource, Panel, Tabs, PreText, Div, Legend, FactorRange, RadioButtonGroup
from bokeh.models import CheckboxButtonGroup
from bokeh.models import Button  # for saving data
from bokeh.models.widgets import DataTable, DateFormatter, TableColumn
from bokeh.models import HoverTool
from bokeh.plotting import figure

from bokeh.models import ColumnDataSource, CustomJS
from bokeh.models.widgets import Button

doc = curdoc()


class ViewModelDummy:
    n_rows = 8
    n_cols = 8

    def __init__(self):
        self.cds_table = ColumnDataSource({})
        self.cds_plot = ColumnDataSource({})
        self.plot_view = figure(title="Basic Title", width=400, height=300,
                                x_range=FactorRange(*self.get_columns(ViewModelDummy.n_cols)), toolbar_location=None)
        self.table_view = DataTable(width=400, height=120, sortable=False, autosize_mode="none", source=self.cds_table,
                                    columns=self.get_table_cols(ViewModelDummy.n_cols))

        self.plot_glyph = None

    def init(self):
        self.init_data()
        self.plot_glyph = self.plot_view.vbar_stack(self.get_index(ViewModelDummy.n_rows), x='index', width=0.9,
                                                    alpha=0.5,
                                                    source=self.cds_plot, )

        self.plot_view.x_range.range_padding = 0.1
        self.plot_view.xaxis.major_label_orientation = 1
        self.plot_view.xgrid.grid_line_color = None
        return self

    def init_data(self):
        self.cds_table.data = ColumnDataSource.from_df(self.get_df())
        self.cds_plot.data = ColumnDataSource.from_df(self.get_df().transpose().reset_index(drop=False))

    def update_data(self, magnitude):
        df = self.get_df(magnitude)
        df_t = df.transpose().reset_index(drop=False)
        self.cds_plot.data = ColumnDataSource.from_df(df_t)

    def get_layout(self):
        return column(self.plot_view, self.table_view)

    def get_df(self, offset=1):
        data = np.random.rand(ViewModelDummy.n_rows, ViewModelDummy.n_cols)
        data[0,:] = offset #init first row with offset to ensure scaling is working as intended
        return pd.DataFrame(data,
                            columns=self.get_columns(ViewModelDummy.n_cols),
                            index=self.get_index(ViewModelDummy.n_rows))

    def get_columns(self, n_cols):
        return ["col" + str(i) for i in range(n_cols)]

    def get_index(self, n_rows):
        return [str(i + 10) for i in range(n_rows)]

    def get_table_cols(self, n_cols):
        return [TableColumn(field=col, title=col, width=100) for col in self.get_columns(n_cols)]


class ViewModelManager:
    def __init__(self, view_models):
        self.view_models = view_models
        self.button1_update_data = Button(label="Update Data Magnitude 1")
        self.button2_update_data = Button(label="Update Data Magnitude 10")

    def init(self):
        self.button1_update_data.on_click(self.update_data_magnitude_10)
        self.button2_update_data.on_click(self.update_data_magnitude_100)

        for vm in self.view_models:
            vm.init()

    def update_data_magnitude_10(self):
        self.view_models[0].update_data(1)  # update only first plot
        # update all plots
        # for vm in self.view_models:
        #    vm.update_data(0)

    def update_data_magnitude_100(self):
        self.view_models[0].update_data(10)  # update only first plot
        # update all plots
        # for vm in self.view_models:
        #    vm.update_data(10)

    def get_layout(self):
        n_cols_data = 2

        layout_col_left = column(self.button1_update_data, self.button2_update_data)

        layout_sub_columns_right = [[self.view_models[i].get_layout() for i in range(len(self.view_models)) if
                                     i % n_cols_data == j] for j in range(n_cols_data)]

        layout_col_right = column(row([column(col_d) for col_d in layout_sub_columns_right]))

        return row(layout_col_left, layout_col_right)


n_vm = 50
view_models = [ViewModelDummy() for i in range(n_vm)]
view_manager = ViewModelManager(view_models)
view_manager.init()

doc.add_root(view_manager.get_layout())

Stack traceback or browser console output

No response

Screenshots

bokeh_layout_invalidation

@bryevdv
Copy link
Member

bryevdv commented May 15, 2022

If the current amount of space is not sufficient to accommodate ticks, then more space has to be allocated and everything has to be re-painted. I don't think there is any way to avoid this in general. If this is a problem, e.g. because you have 50 plots on a single page, then the typical specific optimizations are some combination of:

  • pre-allocating extra space for anticipated tick lengths by setting the appropriate min_border properties
  • using more compact formatting for tick labels (e.g. 2.4B instead of 2400000000)
  • rotating the tick labels to take up a fixed amount of space in the direction normal to the plot axis

cc @bokeh/core for any thoughts but I am incline to close this with no action.

@bryevdv
Copy link
Member

bryevdv commented May 15, 2022

Also, for bug reports on "dev" releases, please only report using the latest dev release (3.0.0dev6 at this time)

@muendlein
Copy link
Contributor Author

If the current amount of space is not sufficient to accommodate ticks, then more space has to be allocated and everything has to be re-painted. I don't think there is any way to avoid this in general. If this is a problem, e.g. because you have 50 plots on a single page, then the typical specific optimizations are some combination of:

  • pre-allocating extra space for anticipated tick lengths by setting the appropriate min_border properties
  • using more compact formatting for tick labels (e.g. 2.4B instead of 2400000000)
  • rotating the tick labels to take up a fixed amount of space in the direction normal to the plot axis

cc @bokeh/core for any thoughts but I am incline to close this with no action.

Pre-allocating extra space for the axis is the desired option for me however this functionality seems unsupported. Using the min_border property on the plot does not have any impact on this issue and there seem to be no option to set an explicit y axis width. Am I overlooking something here?

I am not a web expert but why does everything has to be repainted when the figure widget has a fixed size and only one widget is updated? That seems quite unnatural, I'd rather expect that everything inside the figure widget is rescaled leaving the rest of the layout untouched.

@bryevdv
Copy link
Member

bryevdv commented May 16, 2022

cc @mattpap to discuss layout issues, and possible changes still yet to land with upcoming 3.0 release

I will also say that Bokeh CDS are not lightweight objects. If you are ending up with 100 of them on a single page then Bokeh simply may not be the right tool for your use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants