holoviz / param

Param: Make your Python code clearer and more reliable by declaring Parameters
https://param.holoviz.org
BSD 3-Clause "New" or "Revised" License
427 stars 73 forks source link

Make param.depends efficient #927

Open MarcSkovMadsen opened 6 months ago

MarcSkovMadsen commented 6 months ago

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.

https://github.com/holoviz/param/assets/42288570/c814b45f-d3e7-419f-8392-6c2e89930c4e

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.

hoxbro commented 6 months ago

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 commented 6 months ago

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.

maximlt commented 6 months ago

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

philippjfr commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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