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

Counting number of scene.play and wait calls #214

Closed
huguesdevimeux opened this issue Jul 23, 2020 · 15 comments
Closed

Counting number of scene.play and wait calls #214

huguesdevimeux opened this issue Jul 23, 2020 · 15 comments
Labels
help wanted We would appreciate help on this issue/PR

Comments

@huguesdevimeux
Copy link
Member

huguesdevimeux commented Jul 23, 2020

I'm refactoring the progressbar and t-values generation. See #183

I'm facing an issue ; I would like to be able to count the number of calls of scene.play or scene.wait in construct() defintion, because I need to know when is the last animation.

As far as I know there is no way to do it in manim, as right now manim just takes scene.play and scene.wait one after the other (so it does not need to get the total number of scene.play and scene.wait calls.

So, to do that, I think the only solution is to disassemble construct function to get the number CALL_FUNCTION instances with scene.play and scene.wait. Something like this https://stackoverflow.com/a/16042229/12009882

But my question is, is it good? Like, is it ok to have something like this? Because this method is slightly too much hacky in my mind (although it would work and is not very complicated).

Maybe this is totally fine and I'm just nitpicking, please tell me :D

EDIT : Looking for CALL_FUNCTION in function's instructions (disassembled) won't work as the name of the function does not appear. Instead, we can look for LOAD_METHOD with argval 'wait' or 'play'.
Example :

Instruction(opname='LOAD_METHOD', opcode=160, arg=3, argval='wait', argrepr='wait', offset=22, starts_line=None, is_jump_target=False)

@huguesdevimeux huguesdevimeux changed the title Counting number of self.play and wait calls Counting number of scene.play and wait calls Jul 23, 2020
@leotrs
Copy link
Contributor

leotrs commented Jul 24, 2020

Instead of disassembling a function, just keep an instance member that counts the number of times these have been called.

class Scene:
    def play(self, ...):
        ...
        self._play_calls += 1

@huguesdevimeux
Copy link
Member Author

I know, but I need to get the number of play and wait calls before they are called (i.e before scene.construct() is called.
(Btw the variable you proposed already exists and is scene.num_plays).
What I need to do is to get if the play/wait call is last one of the scene. I thought to do something like (scene.num_plays == scene.total_num_plays= to check if this is the last call. Then I would use disassembling to get scene.total_num_plays.

On second thought, I don't think that using disassembling is not orthodox. I think I will use that, if no one objects :D.

@leotrs
Copy link
Contributor

leotrs commented Jul 24, 2020

I think it's fair game, though the inspect module in general tends to be a bit slow. Would be interesting to see if there are any speed changes after your PR.

@huguesdevimeux
Copy link
Member Author

That's a problem.

But there is another that I didn't think about: when play and wait are called from outside the construct() (e.g through a nonlocal function), just disassembling construct won't work as it is not recursive by default.
I remember facing this issue in #166 when I had to implement serialization of functions, and I had to wrote something recursive which was very slow and not viable.

So I don't think its possible to count number of play and wait. I will find a workaround.
I will close this in a few days in no one has an idea.

@leotrs
Copy link
Contributor

leotrs commented Jul 24, 2020

One workaround (that would require a major rewrite of some parts of manim) is to implement scene rendering in a lazy fashion allowing for dry runs. For example, you can run all of deconstruct without actually rendering anything, just changing the coordinates/colors/etc of your abstract mobjects. After that dry run, you can easily get the count of play/wait, as well as many other Things About the Scene that may be interesting on their own accord. After that you can actually do the rendering. BTW, such a system might also help the scene caching stuff.

@huguesdevimeux
Copy link
Member Author

This is a brilliant idea, but IMO this is too much work for what we need. Unless there are other projects of features that would need this kind of thing, I think this is worthless.

BTW, such a system might also help the scene caching stuff.

And could you clarify this point ? I don't see why this could improve caching.

@huguesdevimeux huguesdevimeux added the help wanted We would appreciate help on this issue/PR label Jul 24, 2020
@leotrs
Copy link
Contributor

leotrs commented Jul 25, 2020

If we had such an abstract system detached from the rendering system, we could run the old Scene and the new Scene without rendering them to determine whether they are the same. If they are, then don't render the new one.

@leotrs
Copy link
Contributor

leotrs commented Jul 25, 2020

But I agree that would require a major rewrite of basically everything..

@huguesdevimeux
Copy link
Member Author

huguesdevimeux commented Jul 26, 2020

If we had such an abstract system detached from the rendering system, we could run the old Scene and the new Scene without rendering them to determine whether they are the same. If they are, then don't render the new one.

I see. This is pretty much what it does in scene caching.

But I agree that would require a major rewrite of basically everything..

In fact this could be easily done with a decorator placed for wait and play, that would wether play or wait are called.
We could call two times construct(), first time while doing a dry run so the decorator just counts, and second time it would go normally.
But this would cause trouble when there are heavy functions used in construct().
Or we could do like save the arguments that were passed to play and wait , to execute play and wait later with these arguments. We then won't need to do construct() twice. The question is, how to do it? How to save arguments? Maybe with pickle, something like that.

@leotrs
Copy link
Contributor

leotrs commented Jul 26, 2020

Ugh let's not pickle things unless absolutely necessary :)

@huguesdevimeux
Copy link
Member Author

Ugh let's not pickle things unless absolutely necessary :)

Then I don't see how we could do without calling scene.construct() (one as a dry_run). IMO this is a bad idea when it needs to call other heavy functions.

@kilacoda-old
Copy link
Contributor

What's the problem with using inspect.getsource(SceneClass) and counting the occurrence of self.play and self.wait?

@huguesdevimeux
Copy link
Member Author

@kilacoda I first tought to do this only for construct(), but as obvious as it may seem I didn't think to do it with the whole SceneClass lol, I'm stupid. So thank you very much for bringing up this idea.

The issue was/is that if these calls are outside the construct, then we won't be able to detect them (except by doing a weird recursive thing that would take long). We should not have this issue if we take the source (or the disassembly) of the whole class as all the wait/play calls are supposedly within the latter.

@huguesdevimeux
Copy link
Member Author

@kilacoda On second thought it won't work when there is a loop. Like, manim will be trick into finding only one self.play when there is multiple invocation because of the loop.

@huguesdevimeux
Copy link
Member Author

So I think this is impossible (without rewriting the whole codebase)
Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We would appreciate help on this issue/PR
Projects
None yet
Development

No branches or pull requests

3 participants