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

Optimized various point-related :class:.VMobject methods #3292

Open
wants to merge 51 commits into
base: main
Choose a base branch
from

Conversation

chopan050
Copy link
Contributor

@chopan050 chopan050 commented Jul 16, 2023

Overview: What does this pull request change?

Main changes:

  • Aggresively removed every call to np.append in VMobject's methods, and reduced the use of VMobject.append_points as much as possible, because their extensive use is expensive. When possible, they're replaced with preallocations of empty ndarrays whose lengths can be precalculated.
  • Modified VMobject.append_points to use np.empty instead of np.append, because it is slightly faster this way. Useful because this method can still get called a lot.
  • Removed calls to np.linspace from VMobject's methods, because repeated call to this method for small arrays is also too expensive.
  • Added a new attribute VMobject.bezier_alphas related to the nppcc. This attribute is a precalculation of the alphas needed to interpolate pairs of anchors to obtain the required handles for the new straight segments that shall be represented by Bézier curves. This replaces those special cases where np.linspace was originally used for interpolation.
  • Optimized VMobject.consider_points_equals by replacing the np.allclose method, which is also really expensive when used repeatedly to compare only two 3D points.
    • It is now implemented in a similar fashion to VMobject.consider_points_equals_2d, where a comparison coord-by-coord is made to determine if the points are close or not.
    • Both functions now consider two separate cases: when there's a single pair of points p0 and p1 (return a bool), and when there are multiple pairs of points (return a boolean ndarray).
      • Both implementations are much faster than the original np.allclose approach.
    • NOTE: I removed the "rtol". Let's discuss whether this is the best decision, please.
  • Added two new methods, VMobject.consider_points_different and VMobject.consider_points_different_2d, because more often than not we actually want to know if two points are distant enough from each other, rather than if they're close to each other. EDIT: Nevermind, I removed those methods to avoid code duplication.
  • Added new methods VMobject.get_subpath_split_indices and VMobject.get_subpath_split_indices_from_points, which instead of explicitly obtaining a Python list of subpaths, it obtains an (n_subpaths, 2) ndarray of indices indicating where every subpath starts and ends.
    • These methods also accept parameters n_dims (which allows us to choose between 2D or 3D point comparison) and strip_null_end_curves (necessary for VMobject.align_points where the null end curves at every subpath must be removed as a fix to certain bug).
  • Rewrote VMobject.get_subpaths_from_points and VMobject.gen_subpaths_from_points_2d to use this new method VMobject.get_subpath_split_indices_from_points.
    • VMobject._gen_subpaths_from_points is no longer used and can probably be deprecated if this PR is accepted.
  • Some methods were completely rewritten:
    • VMobject.add_points_as_corners: it originally used a for loop to add every point one by one through VMobject.add_line_to. This was extremely expensive (especially with the previous use of np.linspace) and was completely changed to just add all the points at once, calculating the necessary handles via interpolations.
    • VMobject.change_anchor_mode: if using "jagged" mode, it is not necessary at all to separate the points into subpaths (a simple interpolation is sufficient). Only get the subpaths in "smooth" mode. In this case, the new implementation now uses this new method VMobject.get_subpath_split_indices instead of explicitly precalculating all the subpaths previously and storing them all in a Python list. As the number of points remains pretty much the same and only the handles change, there's no need to clear the points or create another ndarray of points: we can just directly overwrite the handles.
    • VMobject.align_points: this method was completely rewritten so that it now uses VMobject.get_subpath_indices too. In this way, one can also easily calculate useful information like how long is every subpath (just subtract the pairs of indices!) and, thus, which subpath is longer when comparing pairs of subpaths between the two VMobjects (via np.maximum). The np.appends were obliterated. As mentioned earlier, this new implementation uses strip_null_end_curves=True to remove the excess null curves at the end of the subpaths, if any.
  • Introduced the concept of Mobject.memory, which might be useful for memoizing certain desirable properties.
    • Used it at VMobject.point_from_proportion.
      • My main motivation was that I first managed to optimize (via binary search through the accumulated Bézier lengths) the process of finding the Bézier curve from where to calculate the point from the given proportion. This would mean going from an O(n) algorithm to an O(log(n)) algorithm... except that it still needed to calculate all the Bézier curve lengths in O(n) operations. If only we didn't need to calculate them...
      • Created two new methods, VMobject._init_curve_memory and VMobject._update_curve_memory, which precalculate the lengths and accumulated lengths of the Bézier curves making up the VMobject and update it only if the actual points or the number of sample points per curve have changed.
      • This all resulted in a huge improvement in VMobject.point_from_proportion's time!
      • Didn't go much further with this idea because I first wanted your opinions about this. There are more potential applications for this idea. For example, there's a method in ManimGL, VMobject.get_area_vector, where this could be used as well.
  • Some other (minor?) changes.

Motivation and Explanation: Why and how do your changes improve the library?

My original intention was to optimize my test scene where I have 110 ParametricFunctions being updated in a span of 5 seconds, which is really expensive!

from manim import *

class LaplaceScene(Scene):
    def construct(self):
        axes = Axes(x_range=[-1, 7], y_range=[-1, 4], x_length=16.32, y_length=10.20).add_coordinates()
        s = ValueTracker(0)
    
        def get_min_x(k):
            if k > 3:
                return np.floor(2*np.log(k/3.5)/0.5) / 2 - 0.5
            if k < 0:
                return np.floor(2*np.log(-k/0.5)/0.5) / 2 - 0.5
            return -0.5

        draw_horizontal = lambda k: axes.plot(
            lambda t: k*np.exp(-s.get_value()*t), x_range=[get_min_x(k), 6.5, 0.5],
            stroke_width=1.0, stroke_color=RED_A,
            use_vectorized=True,
        )
        horizontals = {k: draw_horizontal(k) for k in range(-15, 96)}
        del horizontals[0]

        verticals = {
            k: Line(
                axes.c2p(k, -1), axes.c2p(k, 5),
                stroke_width=1.0, stroke_color=BLUE_A,
            )
            for k in range(1, 7)
        }

        draw_func = lambda: axes.plot(
            lambda t: t*np.exp(-s.get_value()*t), x_range=[0, 6.5, 0.5],
            stroke_color=YELLOW,
            use_vectorized=True,
        )
        func = draw_func()

        self.play(
            *[Create(verticals[key]) for key in verticals],
            *[Create(horizontals[key]) for key in horizontals],
            DrawBorderThenFill(axes),
        )
        self.play(Create(func), run_time=2)

        func.add_updater(lambda plot: plot.become(draw_func()))
        for k in horizontals:
            (lambda k: horizontals[k].add_updater(
                lambda line: line.become(draw_horizontal(k))
            ))(k)

        self.play(s.animate.set_value(0.5), run_time=5)

        func.clear_updaters()
        for k in horizontals:
            horizontals[k].clear_updaters()

        self.wait()
        self.play(VGroup(axes, func, *horizontals.values(), *verticals.values()).animate.scale(0.5))
        self.wait()
        
with tempconfig({"preview": True, "quality": "medium_quality", "disable_caching": True}):
    scene = LaplaceScene()
    scene.render()

Almost half of the time of the scene is spent on ParametricFunction.generate_points, and there are 3 major culprits: VMobject.make_smooth, VMobject.add_points_as_corners, and Axes.coords_to_point.

imagen

In #3281 I optimized get_smooth_handle_points (and other Bézier-related functions), which was an important cause of VMobject.make_smooth's excessive time, and in #3285 and #3286 I addressed the issue with Axes.coords_to_point and, more specifically, NumberLine.number_to_point.

Regarding the VMobject.add_points_as_corners and the VMobject.get_subpaths, which are the other major issues, I initially rewrote ParametricFunction to circumvent those issues. After all, I don't think ParametricFunction.generate_points should be relying a lot on these functions to, well, generate its points, when they can be generated in other efficient ways. I have some ideas about rewriting ParametricFunction, which I wanna discuss in a future PR.

However, there are still many other VMobject subclasses which depend on those currently inefficient methods too. As I noticed this was a general issue for many VMobjects, I decided to address the root issues and directly modify and optimize most of VMobject's point-related methods.

EDIT: here's a new screenshot of how ParametricFunction.generate_points looks after these changes. VMobject.get_subpaths (replaced by VMobject.get_subpath_split_indices) and VMobject.add_points_as_corners are no longer a problem!
imagen

Links to added or changed documentation pages

Further Information and Comments

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

chopan050 and others added 30 commits July 8, 2023 02:42
@chopan050
Copy link
Contributor Author

Done! I addressed most comments.
One thing I forgot to mention is regarding VMobject.insert_n_curves_to_point_list:

    def insert_n_curves_to_point_list(self, n: int, points: np.ndarray) -> np.ndarray:
        # ... other stuff
        for quad, sf in zip(bezier_quads, split_factors):
            if sf == 1:
                new_points[start_i : start_i + nppcc] = quad
                start_i += nppcc
            else:
                for i in range(sf):
                    new_points[start_i : start_i + nppcc] = partial_bezier_points(
                        quad, i / sf, (i + 1) / sf
                    )
                    start_i += nppcc
        return new_points

In #3281 I added a new function in beziers.py called subdivide_bezier, and mentioned that if that PR gets approved it would be a good idea to replace the use of partial_bezier_points in the above code with subdivide_beziers:

    def insert_n_curves_to_point_list(self, n: int, points: np.ndarray) -> np.ndarray:
        # ... other stuff
        for quad, sf in zip(bezier_quads, split_factors):
            new_points[start_i : start_i + sf * nppcc] = subdivide_bezier(quad, sf)
            start_i += sf * nppcc
        return new_points

This would look much cleaner and is also more efficient, since in partial_bezier_points we're splitting the Béziers twice to get the desired portion in the middle (discarding the start and end portions, if any), so using that function in VMobject.insert_n_curves_to_point_list is double the necessary work. On the other hand, with subdivide_bezier the work is done only once and all the pieces are used, no portion is discarded.

So, if #3281 gets merged first, I'd wanna add this change to this PR before merging this one.

@behackl behackl modified the milestones: v0.18.1, v0.19.0 Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🆕 New
Development

Successfully merging this pull request may close these issues.

4 participants