Scene Caching must recognise changes in attributes of objects

[Aathish04]

Currently, the Scene Caching Mechanism considers three hashes to check if a scene and its contents have changed:

1. hash corresponding to the arguments passed to scene.play (or scene.wait).
2. hash obtained with all the mobjects present in scene, including those not in the current play/wait call
3. hash obtained from camera_config.

However, using just these three hashes is not specific enough in some contexts.

Consider the following test scene:

class Test(Scene):
    def construct(self):
        T1 = TextMobject("M")
        self.play(Write(T1))

After the first render, if the T1 TextMobject is changed to some other letter, say Q, the scene caching mechanism will not recognise this change, and will still show the original animation of the letter M being written.

A possible solution to this would be to also hash the attributes of all the mobjects present in the Scene, or at least those mobjects being animated, and add that to the list of hashes used.

cc-ing @huguesdevimeux

EDIT : Original issue :
Okay, so this works as intended for the first render, and produces the video as required, but doesn't re-render if the Vector Field is changed.

Let's take the same script I used in #302 :

from manim import *

class TestVectorField(Scene):
    def four_swirls_function(self,point):
        x, y = point[:2]
        result = ((y**3 - 4 * y) * RIGHT + (x**3 - 16 * x) * UP)*0.05
        return result

    def construct(self):
        lines = StreamLines(self.four_swirls_function)
        self.add(AnimatedStreamLines(
            lines,
            line_anim_class=ShowPassingFlash
        ))
        self.wait(2)

It's output should be something like:

If I change four_swirls_function to something else, like result = ((y**2 - 4 * y) * RIGHT + (x**3 - 16 * x) * UP)*0.05,
the output should be something like:

But, the scene caching mechanism still uses the previous cached file.

EDIT: I don't think this is due to the bug that this PR fixes, but a more specific case of what I described in #320.

Originally posted by @Aathish04 in #315 (comment)

[huguesdevimeux]

Thanks for the report, and sorry for the answer delay !

The bug you described is super weird. In theory, scene-caching mechanism takes everything (see hashing.py, and more particularly CustomEncoder.default). So this :

A possible solution to this would be to also hash the attributes of all the mobjects present in the Scene,

is already done by manim.

I don't see rn where this comes, but I will take a look at that and try to fix it in a few days.

[Aathish04]

Thanks for the report, and sorry for the answer delay !

The bug you described is super weird. In theory, scene-caching mechanism takes everything (see hashing.py, and more particularly CustomEncoder.default). So this :

A possible solution to this would be to also hash the attributes of all the mobjects present in the Scene,
is already done by manim.

I don't see rn where this comes, but I will take a look at that and try to fix it in a few days.

I implemented such a thing in already. I'm testing it rn :)

[huguesdevimeux]

What do you mean ? What thing ?

[Aathish04]

Ok, so currently, the scene caching mechanism takes only the dict of the Animation that's being played/rendered, and does not consider the dict of the Mobjects in the animation.

I have added some code that makes the mechanism recursively get the dict of any objects in it that have a dict.

This way, it will also account for the Attributes of the Mobjects in the animation.

[Aathish04]

@huguesdevimeux Update:

So my changes fix this issue, but cause a RecursionError when testing with the script in #315

I believe this is because there are so many StreamLines in the AnimatedStreamLines that going through the dicts of each and every one of them occurs more than 1000 times.

Thoughts?

[huguesdevimeux]

Ok, so currently, the scene caching mechanism takes only the dict of the Animation that's being played/rendered, and does not consider the dict of the Mobjects in the animation.

it does already if I understand correctly. See l.146 hashing.py.

I have added some code that makes the mechanism recursively get the dict of any objects in it that have a dict.

And it does already as well, as json.dumps is by essence recursive.

EDIT : just saw your message.

It would be better if I had access to the code of what you did. Could you share me the branch ? then we will be able to discuss that more deeply (in a few days tho, I can't right now :D)

[Aathish04]

Ok, so currently, the scene caching mechanism takes only the dict of the Animation that's being played/rendered, and does not consider the dict of the Mobjects in the animation.

it does already if I understand correctly. See l.146 hashing.py.

I have added some code that makes the mechanism recursively get the dict of any objects in it that have a dict.

And it does already as well, as json.dumps is by essence recursive.

Nope. I believe it doesn't do that because of:

manim/manim/utils/hashing.py

Lines 71 to 75 in 35e9e05

	def key_to_hash(key):
	if not isinstance(key, (str, int, float, bool)) and key is not None:
	# print('called')
	return zlib.crc32(json.dumps(key, cls=CustomEncoder).encode())
	return key

Here, it basically ignores any key in dict if it has a string as its key.

In the __dict__ of the animation (which is obj), the Mobject being animated has the string key mobject, so it, and by extension, it's attributes, are ignored.

[huguesdevimeux]

Here, it basically ignores any key in dict if it has a string as its key.

It does not ignore it. It let it as it is, and then it's hashed.

To check if the issue is related to that, could you check if the bug with TextMobjext is happening on master ?

I think this is related to the try expect thing, maybe.

Not sure tho. I would love to see what you did !

[Aathish04]

To check if the issue is related to that, could you check if the bug with TextMobjext is happening on master ?

Yes, the issue with the TextMobject happens on Master too. I tried after pulling from Master today, it does work on Master.

I'll push my changes to a new branch in about 2 mins.

[Aathish04]

@huguesdevimeux You can see my changes over here : Aathish04@bbf43fc

[Aathish04]

@huguesdevimeux

It does not ignore it. It let it as it is, and then it's hashed.

Yea, the problem is that it doesn't hash the attributes of the value of mobject...

The hashing mechanism just saw that there was a TextMobject there, and assumed it was the same, and used the Cached file , not recognising that the actual attributes of the TextMobject changed.

The changes I made sort of fix that, but I can't get it to work on the AnimatedStreamLines example I sent, since it throws a recursion error, probably because there are too many StreamLine mobjects to check.

[Aathish04]

@huguesdevimeux Some Updates:

Commit Aathish04@b90218c fixes the RecursionError, but the problem I mentioned in #302 (the second problem, where the same cached file is used even if the scene is changed), still persists. At this point I think that it might be unrelated to this issue 🤔 .

Wierdly, if I add a keyword argument to the AnimatedStreamLines(), any keyword argument, the hash changes and it regenerates the cached video file, which means it is checking the attributes of the objects, but can't recognise a change in the four_swirls_function, for some reason...

EDIT: I think know this is because the hash of a function remains the same even if the function itself is changed:

>>> hash(lambda x:x**2)
283370352
>>> hash(lambda x:x**4)
283370352

So, if that is the reason why, we need to work in a way to check that the functions have been changed or not, perhaps using inspect?

Also, I was unable to reproduced this bug in the latest master with TextMobject, which was pretty wierd.

EDIT TO EDIT:

It indeed was because the hash of a function remained the same even if the function itself was changed. To go around this, if a method is encountered, it is replaced with its source code when hashing in Aathish04@d87d047 .