Skip to content

Deprecate different signatures for updaters #321

Open
@leotrs

Description

@leotrs

The problem

This is part of mobject:mobject.update() method:

def update(self, dt=0, recursive=True):
if self.updating_suspended:
return self
for updater in self.updaters:
parameters = get_parameters(updater)
if "dt" in parameters:
updater(self, dt)
else:
updater(self)

L159 fetches the parameters of each updater, each time the object is updated, which may occur multiple times in a single animation, let alone a whole scene. Fetching the parameter list makes use of the inspect library, which is not the fastest thing in the world.

I think this is super unnecessary and adding a lot of overhead. This checking of the parameter list is done so that each updater can be called appropriately: either updater(self, dt), or just updater(self). The gain in convenience for the end user seems very small in comparison to the overhead that this incurs in.

A possible fix

My suggestion is that we should just make each updater always accept two arguments, and never check the parameter list. The chunk of code above could just become

    def update(self, dt=0, recursive=True):
        if self.updating_suspended:
            return self
        for updater in self.updaters:
            updater(self, dt)

No parameter fetching necessary.

Incidentally, the current version forces each updater function to name its second parameter dt, if it has one. With my proposal, updater functions' second argument may be called whatever the user wants. If an updater function doesn't need to use the second argument, they can just ignore it.

Example

Here's a scene.

from manim import Scene, Integer, VGroup, Transform

def dummy_updater(obj, dt):
    return [i**2 for i in range(100)]

class TestScene(Scene):
    def construct(self):
        number = Integer(0)
        for _ in range(1000):
            number.add_updater(dummy_updater)

        self.add(number)
        for i in range(10):
            self.play(Transform(number, Integer(i)))

When executed with the current master, I get

$ rm -r media/
$ time manim -l scene.py 
...suppressed manim output...
real	1m7.297s
user	0m57.463s
sys	0m2.940s

When ran with my suggestion:

$ rm -r media/
$ time manim -l scene.py 
...suppressed manim output...
real	0m43.912s
user	0m40.581s
sys	0m2.781s

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementAdditions and improvements in general

    Type

    No type

    Projects

    Status

    🆕 New

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions