Skip to content

New Feature: IPython %%manim magic #943

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

Merged
merged 17 commits into from
Jan 28, 2021

Conversation

behackl
Copy link
Member

@behackl behackl commented Jan 17, 2021

Motivation

See #895.

Overview / Explanation for Changes

This PR introduces a manim magic (works both as a line and as a cell magic). The logic for parsing the passed arguments is (up to a minor modification) using the same, already existing functions that our CLI uses. (Consistency between the interfaces is thus ensured.)

This is how it currently looks like:

image

Steps to try it out locally:

  1. Make sure your Python installation has jupyter(lab) installed. If in doubt, install via [poetry run] pip install notebook.
  2. Start the notebook server ([poetry run] jupyter notebook), and open a (new) worksheet (with a Python 3 kernel).
  3. That's pretty much it, make sure to import manim in some way (from manim import * is not required, no further function call is required; the necessary code to setup the %%manim magic is run when the library is imported).
  4. EDIT by @kolibril13 : in case that you use anaconda, do the following to install your maim-environment as a kernel to your notebook:
  • check out the jupyter-magic branch (e.g. gh pr checkout 943)
  • run conda install -c anaconda ipykernel
  • run python -m ipykernel install --user --name=manim-env were you replace manim-env with the name of your environment.
  1. Profit!

Oneline Summary of Changes

- New feature: IPython %%manim magic (:pr:`PR NUMBER HERE`)

Testing Status

I am not sure whether there is a simple way of adding (unit) tests for this interface.

Further Comments

I am very happy that this implementation is as simple as it is; most of it is due to our much improved config system. There are some details with this implementation that I don't really like (but I could not find a way to fix these issues):

  • tqdm (our progress bar) prints to stderr, which causes the red box underneath the code cell. (While rendering, the progress bar will be visible there -- but in the end, when it disappears, the box unfortunately does not disappear with it.)
  • The logging output (which we have a lot of) is pretty annoying (which is why I've set the verbosity to WARNING in the screenshot above (BTW: we should downgrade the creating dummy animation message to INFO or even DEBUG)). What I would have liked was, if the logging output didn't appear there, but in a separate, less intrusive place -- try running from IPython.core.page import page; page("this is where I would have liked the log to show") in a notebook for my suggestion, but I could not manage to move the log there (and I've tried for quite a bit; but I'm also not a IPython expert).
  • I've spared out documentation of the class so far (and I'm not sure how well I'm able to document the ManimMagic class), but I do think that if you are satisfied with this sort of interface, we should reference it prominently in our documentation. (Suggestions for where?)

Closes #895.

Acknowledgements

Reviewer Checklist

  • Newly added functions/classes are either private or have a docstring
  • Newly added functions/classes have tests added and (optional) examples in the docs
  • Newly added documentation builds, looks correctly formatted, and adds no additional build warnings
  • The oneline summary has been included in the wiki

@behackl behackl added the new feature Enhancement specifically adding a new feature (feature request should be used for issues instead) label Jan 17, 2021
@behackl behackl added this to the Release v0.2.1 milestone Jan 17, 2021
@behackl behackl requested a review from kolibril13 January 17, 2021 11:23
@kolibril13
Copy link
Member

kolibril13 commented Jan 17, 2021

Thanks a lot for this!
I am really excited to start manim in this context.

we should downgrade the creating dummy animation message to INFO or even DEBUG

+1 for this

@behackl
Copy link
Member Author

behackl commented Jan 17, 2021

The tests are currently failing because pytest --doctest-modules manim skips, as intended, the import from manim.utils.ipython_magic in __init__.py -- but then heads over to that file anyways to look for tests in the docstrings (and then pytest fails because IPython is not installed).

The "easy" solution would be to add IPython as a dependency of Manim (but I'm not sure that this is necessary). I'm also sure it is possible to tell pytest to ignore the file. And the third option would be to change the code in ipython_magic.py in a similar way to the imports in __init__.py: by pulling the IPython imports into a try block, and move the definition of the class to a subsequent finally block.

I probably have a slight preference for telling pytest to skip the file, but I'd have to find out how this can be done. Any other opinions?

@kolibril13
Copy link
Member

I'm also sure it is possible to tell pytest to ignore the file

+1 for this idea.

Another thing:
Might it be possible to have some jupyter-only comments?
e.g. what I would find really convenient would be a tool that the output video matches the screen pixel-to-pixel.
At the moment all video qualities (low, medium, high) have the same output size, which might not be the most ideal solution.
So I am suggesting the following:

  • A flag -ptp that can be used in the jupyter cell magic to display the output cell pixel-to-pixel.

Some further thoughts on that:

  • As low-quality videos would be then very small, there could be additionally a scaleoutput flag, to expand the output cell e.g. by a factor of 2.
  • It might then be also useful to define a maximum width that the output frame can have

@behackl
Copy link
Member Author

behackl commented Jan 17, 2021

Extensions are welcome, although I'd suggest to move them to follow-up PRs.

The current implementation caps the width of the video frame to 100% of the corresponding container; larger videos are scaled down to fit in. But besides that, the rendered resolution should be displayed.

@kolibril13
Copy link
Member

The current implementation caps the width of the video frame to 100% of the corresponding container; larger videos are scaled down to fit in. But besides that, the rendered resolution should be displayed.

Nice!
I agree to merge this pr soon and then add further ideas to it :)

@behackl
Copy link
Member Author

behackl commented Jan 18, 2021

Regarding caching: I've played around for quite some time now, and I fear that I don't really have a good solution for it. (The problem, which I realize @kolibril13 only reported to me privately: sometimes videos don't update after rendering a cell again.) I've found that the issue is mainly resolved by switching from classic Jupyter notebooks to JupyterLab (at least after a minor modification of the code; I'll push that later) -- which is what I will suggest in the documentation.

@behackl behackl marked this pull request as ready for review January 18, 2021 21:29
@behackl
Copy link
Member Author

behackl commented Jan 18, 2021

This is ready for being reviewed.

I'd like to propose that we move implementation of further features and refinements of this interface to follow-up PRs, if you are happy with the current basic functionality.

Copy link
Member

@kolibril13 kolibril13 left a comment

Choose a reason for hiding this comment

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

Works for me as expected in jupyter lab.
Jupyter notebooks have an issue with updating the videos (chrome and firefox), however, this can be addressed in a future pr (I'll have a look on that in a few days).

@behackl
Copy link
Member Author

behackl commented Jan 22, 2021

I've added one additional commit, 0dbd067 -- this is a suggestion how the caching problem could be solved, at the cost of (intelligently managed) video files in a jupyter directory inside the media directory.

@kolibril13, could you please test whether this resolves the problem with caching for both JupyterLab and classic Jupyter notebooks? (For me it works, but I'd like to make sure.)

(If it doesn't work as intended, we can simply revert the commit.)

@kolibril13
Copy link
Member

@kolibril13, could you please test whether this resolves the problem with caching for both JupyterLab and classic Jupyter notebooks? (For me it works, but I'd like to make sure.)

Works for me as well! :) Thanks for your efforts!

Interesting timing test:
a 3 seconds video with medium quality (run multiple times to reduce the error):

  • With terminal and IDE:
    --- 2.1 seconds ---

  • With jupyter notebook:
    CPU times: user 589 ms, sys: 121 ms, total: 711 ms
    Wall time: 1.01 s

  • Same with jupyter lab:
    CPU times: user 673 ms, sys: 144 ms, total: 817 ms
    Wall time: 1.22 s
    Note, that the import statement was in another cell then the Scene code, which might cause this speed up.
    Here is the code for reference:

from manim import *
%%time
%%manim Example -m -p -v WARNING --disable_caching



class Example(Scene):
    def construct(self):
        def phi_func(t):
            phi = phi_max * np.sin(np.sqrt(g / l) * t) - 90*DEGREES
            return phi
        self.camera.background_color = BLACK

        phi_max = 45 * DEGREES
        g = 9.81
        l = 1
        l_dis = 3
        t = ValueTracker(0)

        coordinate = [l_dis * np.cos(phi_func(t.get_value())),
                      l_dis * np.sin(phi_func(t.get_value())),
                      0]

        anchor = Dot()
        mass = LabeledDot(label= "1kg", point=coordinate).set_z(10)
        mass.add_updater(lambda x : x.move_to([l_dis * np.cos(phi_func(t.get_value())),
                                               l_dis * np.sin(phi_func(t.get_value())),
                                               0]))
        line = Line(anchor.get_center(), mass.get_center() , color= BLUE)
        line.add_updater(lambda x : x.become(Line(anchor.get_center(), mass.get_center(),color= BLUE)))
        self.add(anchor,mass, line, mass)
        self.play(t.animate.set_value(2),rate_func=linear , run_time=3)
import sys
sys.executable
import manim.utils.ipython_magic
manim.utils.ipython_magic.__file__

@behackl
Copy link
Member Author

behackl commented Jan 28, 2021

There don't seem to be further discussions regarding this PR, so I'd like to merge this tomorrow morning (in, like, 6-7h) or so. In particular, I'd like to move this forward in order to have some time before the next release for preparing a follow-up PR for

  • improving documentation and visibility,
  • preparing a fresh docker image for spinning up a JupyterLab server directly,
  • adding a sample ipython notebook to the repository which can be run with one click from the README (either via https://mybinder.org/ or google colab).

Further comments and/or input is -- of course, as always -- very welcome!

@behackl behackl merged commit b2abe69 into ManimCommunity:master Jan 28, 2021
@behackl behackl deleted the jupyter-magic branch January 28, 2021 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Enhancement specifically adding a new feature (feature request should be used for issues instead)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a Jupyter cell magic %%manim
3 participants