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

Make param.depends efficient #927

Open
MarcSkovMadsen opened this issue Apr 2, 2024 · 6 comments
Open

Make param.depends efficient #927

MarcSkovMadsen opened this issue Apr 2, 2024 · 6 comments
Labels
type-feature Feature request

Comments

@MarcSkovMadsen
Copy link
Collaborator

MarcSkovMadsen commented Apr 2, 2024

Its becoming more and more clear to me just how inefficient param.depends and param.bind are if you want to depend on them multiple times.

Thus I suggested at HoloViz meeting that for Panel that we start recommending pn.rx(some_func)(args,...) over pn.bind(some_func, args...) because the end usage is the same. But the pn.rx version is much more efficient. But the feedback was that pn.rx was not yet well enough understood.

As an alternative I would suggest making pn.depends (and pn.bind) more efficient by caching results similar to what pn.rx does.

One more argument is that Panel tutorials promotes creating more advanced applications with the DataStore design pattern. And this also promotes depending multiple times on references. The tutorial uses pn.rx. I don't know if Philipp did it for efficiency reasons or just because he could? But using param.depends without watch=True would have led to an inefficient application https://panel.holoviz.org/tutorials/intermediate/structure_data_store.html.

Example

The below example shows how inefficient it is to depend on a param.depends annotated function multiple times.

inefficient.mp4
import param
import panel as pn

pn.extension()

slider = pn.widgets.IntSlider(value=0, start=1, end=2)

class Model(param.Parameterized):
    value = param.Integer(default=1, bounds=(1,10))

    count = param.Integer(default=0)

    @param.depends("value")
    def result(self):
        self.count+=1
        return self.value
    
    @param.depends("result")
    def other_result(self):
        return self.result()
    
class Model2(param.Parameterized):
    value = param.Integer(default=1, bounds=(1,10))
    result = param.Integer()
    other_result = param.Integer()
    
    count = param.Integer(default=0)

    @param.depends("value", watch=True)
    def _update_result(self):
        self.count+=1
        self.result = self.value

    @param.depends("result", watch=True)
    def _update_other_result(self):
        self.other_result= self.result

model = Model()
model2 = Model2()

pn.Row(
    pn.Column(model.param.value, model.param.count, model.result, model.other_result),
    pn.Column(model2.param.value, model2.param.count, pn.pane.Str(model2.param.result), pn.pane.Str(model2.param.other_result))
).servable()

As you can see, you can avoid the inefficiency by using watch=True and updating named parameters. But this makes your code much longer and much more complicated.

@MarcSkovMadsen MarcSkovMadsen added type-feature Feature request TRIAGE User-submitted issue that hasn't been triaged yet. labels Apr 2, 2024
@hoxbro
Copy link
Member

hoxbro commented Apr 2, 2024

Enabling cache with param.depends should be done carefully as a method can have state information not put into the decorator but available in the class itself, making it a very hard problem.

@philippjfr
Copy link
Member

philippjfr commented Apr 2, 2024

Caching param.depends is effectively an absolute no-go for that reason. In the case of pn.bind you can at least have a reasonable expectation that the function is only dependent on the state of the arguments and memoize on those, in the case of pn.depends (especially in a class setting) you could memoize on the explicitly stated dependencies but that still seems quite implicit and that assumption will break in certain scenarios.

@philippjfr philippjfr removed the TRIAGE User-submitted issue that hasn't been triaged yet. label Apr 2, 2024
@maximlt
Copy link
Member

maximlt commented Apr 2, 2024

@MarcSkovMadsen can you quantify how inefficient param.depends is?

@philippjfr
Copy link
Member

philippjfr commented Apr 2, 2024

I mean param.depends itself isn't inefficient, the problem is that it can lead users to write inefficient code. So I'd say it's at least partially a usage and documentation issue. It's perfectly possible to write apps using depends that cache intermediate results on parameters, in fact that's a pattern I use relatively frequently, i.e. instead of doing this:

class Adder(param.Parameterized):
    a = param.Number()
    b = param.Number()

    @param.depends('a', 'b')
    def result(self):
        return self.a+self.b

you write:

class Adder(param.Parameterized):
    a = param.Number()
    b = param.Number()
    result = param.Number()

    @param.depends('a', 'b', watch=True)
    def _calculate(self):
        self.result = self.a+self.b

When using bind on the other hand there is no obvious way to cache the result.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Apr 2, 2024

I mean param.depends itself isn't inefficient, the problem is that it can lead users to write inefficient code. So I'd say it's at least partially a usage and documentation issue. It's perfectly possible to write apps using depends that cache intermediate results on parameters, in fact that's a pattern I use relatively frequently, i.e. instead of doing this:

class Adder(param.Parameterized):
    a = param.Number()
    b = param.Number()

    @param.depends('a', 'b')
    def result(self):
        return self.a+self.b

you write:

class Adder(param.Parameterized):
    a = param.Number()
    b = param.Number()
    result = param.Number()

    @param.depends('a', 'b', watch=True)
    def _calculate(self):
        self.result = self.a+self.b

When using bind on the other hand there is no obvious way to cache the result.

Agree with Philipp. The main problem is that the bound functions are called the same number of times as you depend on them instead of one time.

The examples I provided are the same as Philipp provided. Philipp second example look simple, but if you start having multiple parameters to update it becomes less obvious what goes on. I think its friction that you have to write that extra boiler plate code in every Parameterized class you write instead of just providing a cache=True or cache=('a','b') parameter to param.depends or similar.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Apr 3, 2024

I don't buy the argument of having state that is not-depended on. Right now without concrete examples I believe that is an antipattern.

Pro adding caching I would also argue that

  • There is a huge difference in how parameters and bound functions are treated by Panel. Parameters are displayed using widgets and bound functions using panes. Thus if a bound function is inefficient and I change to a Parameter+watch=True implementation its not only the Parameterized class that needs to change. Its all the places where its used.
  • Caching is not built into Param. Its built into Panel (pn.cache) thus you cannot actually build the caching your self in the Param world. Making it even harder to make your Parameterized classes efficient and optionally using it in Panel.
  • "Modern" frameworks are "React" like. And the ones I've seen in Python does the caching automatically for the user (as well as the "parameter" dependency resolution).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature Feature request
Projects
None yet
Development

No branches or pull requests

4 participants