Skip to content
This repository has been archived by the owner on Jul 14, 2024. It is now read-only.

Celluloid based animation #21 #32

Closed
wants to merge 1 commit into from
Closed

Celluloid based animation #21 #32

wants to merge 1 commit into from

Conversation

maheswarantp
Copy link

Description

Updated the code to utilize celluloid for animation of ekf_mlp_ani.py

Issue

#21

Figures

samples_hist_ekf.5.mp4

Steps

  • Used the Camera class of celluloid to save snapshots of the matplotlob figure
  • Changed the code to use camera.animate instead of matplotlib.FuncAnimation

Checklist

  • Performed a self review of the code
  • Tested successfully on Google Colab for GPU and CPU

@maheswarantp
Copy link
Author

Celluloid uses matplotlib.ArtisticAnimation for animation as compared to matplotlib.FuncAnimation in the earlier version. Necessary changes have been made in this PR to use Celluloid instead.

The issue that @murphyk mentioned /opt/anaconda3/lib/libopenh264.5.dylib' (no such file) may be fixed by the use of celluloid, not 100% sure, as it is built on matplotlib. Performance issues has reduced with celluloid based ekf_mlp. It has been tested on CPU as well as GPU on colab and it takes 4 minutes and 9 minutes respectively (not sure why GPU is slow) for 200 frames. Not sure of the priority, if this is important, I can do more deeper dive and profile the code.

@maheswarantp maheswarantp changed the title celluloid animation #21 Celluloid based animation #21 Apr 14, 2022
@murphyk
Copy link
Member

murphyk commented Apr 22, 2022

Sorry for the delay. Please can you add a global variable that lets the user choose if they want to use Celluloid or the old method. Also, 4 minutes is crazy slow! How long does it take if you don't save the animation? ie is the bottleneck in the EKF code or the visualization? If the former, is jit being used properly (eg with static argnums)?

@gerdm
Copy link
Collaborator

gerdm commented Apr 22, 2022

Since celluloid still uses matplotlib, I'm not sure using this library (and adding it as a dependency) will help us avoid the error.

@gerdm
Copy link
Collaborator

gerdm commented Apr 29, 2022

Hi @maheswarantp,

We'll close this issue. We don't think we need to add a dependency on celluloid at this point.

@gerdm gerdm closed this Apr 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants