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

Add viewport downsample algorithm #6017

Merged
merged 33 commits into from
Jan 30, 2024
Merged

Add viewport downsample algorithm #6017

merged 33 commits into from
Jan 30, 2024

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Dec 8, 2023

Use downsample to only send the data in the viewport to the frontend.

import holoviews as hv
import numpy as np
from holoviews.operation.downsample import downsample1d

hv.extension("bokeh")
downsample1d(hv.Curve(np.random.rand(1000)), algorithm="viewport").opts(xlim=(0, 10))
screenrecord-2023-12-08_19.17.53.mp4

@hoxbro hoxbro added the type: enhancement Minor feature or improvement to an existing feature label Dec 8, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (231cd71) 88.54% compared to head (1de1336) 88.53%.

Files Patch % Lines
holoviews/operation/downsample.py 22.22% 14 Missing ⚠️
holoviews/tests/core/data/test_cudfinterface.py 0.00% 5 Missing ⚠️
holoviews/core/data/cudf.py 66.66% 1 Missing ⚠️
holoviews/core/data/dask.py 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6017      +/-   ##
==========================================
- Coverage   88.54%   88.53%   -0.02%     
==========================================
  Files         314      314              
  Lines       65872    65942      +70     
==========================================
+ Hits        58327    58379      +52     
- Misses       7545     7563      +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hoxbro hoxbro marked this pull request as draft December 11, 2023 10:34
@droumis
Copy link
Member

droumis commented Dec 18, 2023

@hoxbro . This would be a really fantastic addition. I want to make sure that it works for all the relevant use cases that we are encountering right now.

First, it looks like any type of downsampling does not work with subcoordinate_y=True

Code
import holoviews as hv
import numpy as np
from holoviews.operation.downsample import downsample1d
hv.extension("bokeh")

c1 = hv.Curve(np.random.rand(1000), label='c1').opts(subcoordinate_y=True)
c2 = hv.Curve(np.random.rand(1000), label='c2').opts(subcoordinate_y=True)

curves = hv.Overlay(c1*c2).opts(xlim=(5, 10))
downsample1d(curves, algorithm="viewport")
Screen.Recording.2023-12-18.at.3.08.26.PM.mov

@hoxbro
Copy link
Member Author

hoxbro commented Dec 19, 2023

subcoordinate_y=True should now work:

screenrecord-2023-12-19_10.23.57.mp4

@hoxbro
Copy link
Member Author

hoxbro commented Dec 22, 2023

I have added a workaround when having an x limit set on the element (downsample1d(hv.Curve(np.ones(1_000)), algorithm="viewport").opts(xlim=(5, 10))). This is not very elegant, but I can't spend more time on it. I have talked with Philipp, and did not know a better implementation at the top of his head, but maybe he has a better workaround, which at least does not need to have a separate algorithm to handle it.

The problem is the downsampling works by looking at the stream x_range. The first run will be None, meaning the viewport algorithm will send all data to the browser. When the plot shows up in the browser a stream event will trigger and change the x_range to the xlim range. This is not as big of a problem for the other algorithms as they do an actual downsample to the width pixel size (400 by default).

@hoxbro
Copy link
Member Author

hoxbro commented Dec 22, 2023

We can't send an empty plot either, as this will make the y-axis between 0 and 1, didn't notice this before as I was using random data.

So I'm now sending 400 points (200 from the start and 200 from the end) to get an approximation of the range.

@hoxbro hoxbro marked this pull request as ready for review December 22, 2023 11:04
@droumis
Copy link
Member

droumis commented Jan 9, 2024

#6061 is relevant, given that this PR is primarily about slicing

@philippjfr
Copy link
Member

Set the stream value during the stream Callback initialization:

    def initialize(self, plot_id=None):
        super().initialize(plot_id)
        for stream in self.streams:
            msg = self._process_msg({})
            stream.update(**msg)

@hoxbro hoxbro force-pushed the viewport_downsample branch from 3df6134 to d6b654f Compare January 30, 2024 10:32
@hoxbro hoxbro requested a review from philippjfr January 30, 2024 10:59
Copy link
Member

@philippjfr philippjfr left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@hoxbro hoxbro enabled auto-merge (squash) January 30, 2024 13:25
@hoxbro hoxbro force-pushed the viewport_downsample branch from 22e4b5f to 0e9ca5d Compare January 30, 2024 14:50
@hoxbro hoxbro merged commit c800e52 into main Jan 30, 2024
12 checks passed
@hoxbro hoxbro deleted the viewport_downsample branch January 30, 2024 16:44
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
type: enhancement Minor feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants