manim icon indicating copy to clipboard operation
manim copied to clipboard

Fix crash due to bad rounding in BarChart when no y_step was provided and the max y value was close to 0

Open zeFresk opened this issue 5 months ago • 6 comments

Overview: What does this pull request change?

This PR fixes a bug in BarChart that lead to a crash if the user did not provide the y_step (i.e. tick rate, the user provided y_range=[y_min, y_max]) and the y values were very close to 0 (round(max(ys), 2) == 0).

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

When the user only provided the minimum and maximum of the y_range, the code for BarChart was deducing the tick rate for the y axis using the following code:

round(max(self.values) / y_length, 2)

However, this returns 0 if the maximum is small, because it is rounded down to zero. This lead to a division by zero in a np.arrange downstream. This issue can be fixed by replacing the 2 by a computed value.

This PR replaces the hard-coded 2 with $\max\left(\lceil - \log_{10}(x) \rceil, ~ 2\right)$, with $x$ being equal to max(self.values) / y_length (unrounded y_step in the original code). To ensure retro-compatibility, the newly computed value can't go below 2, in the case of large y values.

Reviewer Checklist

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

zeFresk avatar Sep 15 '24 13:09 zeFresk