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

ImageMobjects are not properly hashed #2924

Open
Tracked by #3482
NicoWeio opened this issue Aug 15, 2022 · 6 comments
Open
Tracked by #3482

ImageMobjects are not properly hashed #2924

NicoWeio opened this issue Aug 15, 2022 · 6 comments

Comments

@NicoWeio
Copy link
Contributor

NicoWeio commented Aug 15, 2022

Description of bug / unexpected behavior

I found that certain operations on ImageMobjects do not invalidate the animation cache, that is, they don't change the mobject's hash value.
In consequence, in a typical workflow of repeated renders, Manim sometimes generates videos that do not match the code they were generated with.

This is because the pixel_array is explicitly filtered out of the hashing operation:

KEYS_TO_FILTER_OUT = {
"original_id",
"background",
"pixel_array",
"pixel_array_to_cairo_context",
}

Expected behavior

All (visible) modifications to an ImageMobject should result in its hash being different.

How to reproduce the issue

Code for reproducing the problem
  1. Run the provided code (manim filename.py without a manim.cfg)
  2. Change the alpha value
  3. Run it again
  4. Manim inappropriately re-uses the cached animation.
from manim import *

class TestImageCaching(Scene):
    def construct(self):
        im = ImageMobject('example.png')

        # change here           ↓↓↓
        im.set_color(RED, alpha=0.2)

        self.add(im)
        self.wait()

Additional comments

The most obvious “fix” would be to remove "pixel_array" from the exclude list – which, considering that #2923 works, doesn't break Manim – but I assume it's there for a reason.
Edit: As you can see in the code snipped below, #2923 “works”, because a repr of the (truncated) array is hashed, not all of its values. In other words, this only works if you're lucky.

@kolibril13
Copy link
Member

@huguesdevimeux As you are the caching expert, do you have an idea why pixel_array is in the exclude list ?

@icedcoffeeee
Copy link
Collaborator

If im not mistaken, it may have something to do with the fact that numpy arrays are not hashable.

>>> from functools import cache
>>> cache(lambda i:i)(np.array([1,2,3]))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unhashable type: 'numpy.ndarray'

@NicoWeio
Copy link
Contributor Author

For reference, here's what happens to ndarrays that aren't filtered out:

elif isinstance(obj, np.ndarray):
if obj.size > 1000:
obj = np.resize(obj, (100, 100))
return f"TRUNCATED ARRAY: {repr(obj)}"
# We return the repr and not a list to avoid the JsonEncoder to iterate over it.
return repr(obj)

@huguesdevimeux
Copy link
Member

huguesdevimeux commented Aug 15, 2022

@huguesdevimeux As you are the caching expert, do you have an idea why pixel_array is in the exclude list ?

I think the issue what they were too large, so there is a workaround.

Hope it's clear.

ÉDIT : @icedcoffeeee It's an old piece of code, but I don't think it's related to the hashability of an np array. It's really just a matter of size

ÉDIT 2 : this issue looks interesting, I will definetly take a look when I can !

@MrDiver MrDiver added this to Dev Board Nov 8, 2022
@MrDiver MrDiver moved this to 🆕 New in Dev Board Nov 8, 2022
@huguesdevimeux
Copy link
Member

Hello and sorry, this went off my mind.

I think the most straightforward and logical thing would be to exclude ImageMobject from being cached. The main reason is that such mobject is really computationally expensive to hash, and there is no really workaround around it.

@huguesdevimeux
Copy link
Member

My proposal would be to generate a short random sequence as an, hash for every pixel_array, in order to invalidate the cache systematically. That's the most straightforward and simple solution I can think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🆕 New
Development

No branches or pull requests

4 participants