Skip to content

Conversation

chopan050
Copy link
Contributor

@chopan050 chopan050 commented May 16, 2024

Overview: What does this pull request change?

Related PRs: #3281, #3767

In manim.utils.bezier, in the function is_closed(), repeatedly using np.allclose() to compare only two 3D points each time is actually very expensive and takes a significant part of the total time of get_smooth_handle_points(). So I rewrote it to simply "compare 1st coords, then compare 2nd coords, then compare 3rd coords", which resulted in a 5x speedup.

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 chopan050 added performance pr:easy review There is nothing particular (i.e, it's about a general/small thing) to know for review! labels May 16, 2024
Copy link
Member

@JasonGrace2282 JasonGrace2282 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Might be nice to add a small doctest, if you think it's neccessary.

@chopan050 chopan050 enabled auto-merge (squash) May 20, 2024 03:38
@chopan050 chopan050 merged commit b415604 into ManimCommunity:main May 20, 2024
@chopan050 chopan050 deleted the optimize-bezier-is-closed branch May 20, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance pr:easy review There is nothing particular (i.e, it's about a general/small thing) to know for review!

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants