Manim scene caching does not play well with the -n flag

[leotrs]

Here's a scene

from manim import Scene, Integer, ShowCreation, Transform

class TestScene(Scene):
    def construct(self):
        number = Integer(0)
        self.add(number)
        for i in range(10):
            self.play(Transform(number, Integer(i)))

Problem 1

This scene cannot be rendered with manim -pl test.py -n5,10. That is, if the first time you render it, you try to render only a specific number of animations, it cannot be done. It can be rendered normally without specifying the -n flag: manim -pl test.py.

Problem 2

After successfully rendering with manim -pl test.py, I tried again manim -pl test.py -n5,10, but the whole scene is being rendered. The -n flag is ignored.

Comments

Judging by the output, I think this has to do with scene caching (cc @huguesdevimeux). I recommend that while we are working out the kinks of the scene cachinc system, we add a flag --disable_caching, so that it can still be used even while the caching system is not fully debugged.

UPDATE: even when using --disable_caching, you get the same behavior.

[huguesdevimeux]

Thank you for the report. I will take a look at this ASAP.

There is already a such flag implemented with the same name ;)

[leotrs]

There is already a such flag implemented with the same name ;)

Oh shit didn't realize that!

[huguesdevimeux]

UPDATE: even when using --disable_caching, you get the same behavior.

Then I don't think this is related to scene caching directly. My idea is that this is related to skip_animation, as its functioning had been slightly modified in PR #166 .

[leotrs]

I was using the -n flag correctly about a week ago, and I'm pretty sure that it changed at the time that #166 was merged. In any case, I agree with you I think it has to do with skip_animation.

[huguesdevimeux]

I won't be able to work on it until a couple of days. If you discover a fix, don't hesitate to PR it !

[leotrs]

I believe the issue may be related to the following code

https://github.com/ManimCommunity/manim/blob/704b63e61435ccf9be325935acac532a62870d69/manim/scene/scene.py#L66-L79

On L66, self.original_skipping_status is set to whatever it was in the config. On L79, it is again set to the config.... but the config was set to False in the previous line. So the end result is that both the config and self.original_skipping_status end up being False, instead of actually restoring whatever it used to be. Is this intended? Is this a typo? Does this sound like it might be related to the current issue?

@huguesdevimeux I know you're busy so please feel free to ignore me until you can take a look!

[huguesdevimeux]

As the comment says, these lines are when you run multiple scenes in a row. if you don't resetskip_animation, it will be set to Truefor the next iteration (i.e, the next scene to render, the next time Scene is instanced) file_writer["skip_animation"] would have been set to true as hashed data would have been used. (Note that this is an issue, file_writer_config, as well as all the config stuff, should be constant and never modified by anything within manim. I would like to hear your opinion on that, but here, for example, we could replace that by adding file_writer_config attribute to Scene, so it will be reset everything we instance Scene).

I think that the line 66 is useless, as this is as the end of a scene generation so it won't affect anything, and as original_skipping_status is set again every time by line 66.

By the way, it looks like it's not related to this issue as these lines don't concern self.construct() (not sure though)

The real issue is, I think, up_to_animation that used to be/is in file_writer_config. #166 modified some stuff here (see https://github.com/ManimCommunity/manim/pull/166/files#r467937389). I will explain why I think is the source of the issue :

Before Scene Caching, manim used index names for partial movie files, Basically, first scene got for name 0001.mp4, second 0002.mp4, you get it. Now that manim has scene-caching, in addition to being cooler, manim names partial-movie-files with their respective hash, as you may have noticed. THE PROBLEM IS : To handle -n i,j flag, in the old world, manim used to take all the partial movies whose name was between i and j. (so, if i = 1 et j = 3, then manim would take 0001.mp4, 0002.mp4, etc). But, Hugues, you might say, what did you do to handle the -n flag now that partial movies files names completely changed ?

The answer is simple :

I completely forget to implement that. lol. Here is the bug ! 100% my fault.

(Once again although I'm responsible for this and therefore should be the one in charge of fixing it, feel free to do it if you can)

[leotrs]

Thanks for the explanation.

Note that this is an issue, file_writer_config, as well as all the config stuff, should be constant and never modified by anything within manim. I would like to hear your opinion on that, but here, for example, we could replace that by adding file_writer_config attribute to Scene, so it will be reset everything we instance Scene.

I would like to have the global config dict be immutable, and perhaps add manim.config.modify_config(config_options) and manim.config.restore_config, or something like this, in order to manage modification of the global config from a single point. But we can leave this for the future.

I think that the line 66 is useless, as this is as the end of a scene generation so it won't affect anything, and as original_skipping_status is set again every time by line 66.

I see what you mean. But my point is that file_writer_config["skip_animations"] is never being restored back to its original value. It's always set to False. Am I missing anything here?

By the way, it looks like it's not related to this issue as these lines don't concern self.construct() (not sure though)

I think you're right, I was just confused by L66 and L78, and I thought this was the source of the bug.

I completely forget to implement that. lol. Here is the bug ! 100% my fault.

Lol. I'm glad we found the issue. I know you're busy, but I don't know how to proceed here. What would be the fix?

[huguesdevimeux]

But my point is that file_writer_config["skip_animations"] is never being restored back to its original value.

You're right, and I don't think it is an issue as it is set to false by default at each scene rendering (this flag is not meant to be used by the user in any way).

Lol. I'm glad we found the issue. I know you're busy, but I don't know how to proceed here. What would be the fix?

I'm not that busy, I'm just working on other things (tests) and this is my priority right now. But I will be able to take care of this in a few days, no doubt.

I think we should be smarter than what was implemented before. My idea would be to do some tricks with scene.num_plays in @handle_caching play and @handle_caching_wait: if scene.num_plays is between i and j, then we skip everything to the next iteration, if not, we can continue normally and it should work fine. This is just a draft if you don't see really how to fix I will be able to work on it in a few days.

[leotrs]

You're right, and I don't think it is an issue as it is set to false by default at each scene rendering (this flag is not meant to be used by the user in any way).

If it's never touched by the user, and it's only touched by each Scene instance, then it should be a member of Scene, not in the config. But we can leave that for another issue.