animate matplotlib plots #348

YBCS · 2024-04-23T20:59:08Z

#279 wip fix

we can only use this feature if show_plot_already is set to True

* development: test(annotations): add more assertions feat(annotations): improve docstring feat(annotations): improve for use as embedded annotations chore: to 1.2.13 feat(annotations): extend to allow both miriam and biosimulations styles

sanjayankur31

Looking good. I've added a test and opened a PR for you to add that to your branch:

YBCS#1

Can we also add saving of animations? Looks simple enough?

https://matplotlib.org/stable/users/explain/animations/animations.html#funcanimation

sanjayankur31 · 2024-05-07T11:58:39Z

pyneuroml/plot/Plot.py


    if show_plot_already:
+        if animate:
+            duration = 5


I was wondering if there was a way to make this an argument? Perhaps animate can be a bool or an int, and if it's an int, we use that as the "duration". That way users will be able to set the duration of their animations.

duration varibale is not consistent. "duration" will depend of lots of factors including data calculation and actual length of data to plot and machine configuration. Even If I try to calculate the "hyperparameter" variables to make it perform the animation in required duration, the actuall outcome usually differs.

So, if we say duration "5", that doesn't mean that the animation will be 5s long? 🤔

sanjayankur31 · 2024-05-07T11:59:30Z

pyneuroml/plot/Plot.py

+        if animate:
+            duration = 5
+            num_frames = len(xvalues[0])
+            interval = duration * 1000 / num_frames


we should have a sensible minimum value that the interval can take. Maybe 5?

yes, I will update this.
Interval is simply " Delay between frames in milliseconds", it can be harcoded. Most tutorial I have seen uses 50 or 100 ms.

Cool--yeh, I think 50ms (20fps) is good for a default minimum value.

YBCS · 2024-05-09T17:50:23Z

Can we also add saving of animations? Looks simple enough?

https://matplotlib.org/stable/users/explain/animations/animations.html#funcanimation

Saving animation is simple. But after some testing with the plots in the codebase I have identified some issues.

Animation is actually very expensive and so it takes lots of time. I have encountered plots with >= 10000 (link)
In my machine saving animation plot with data length 1000 can take more almost a minute.
Plot is a generic function, it is probably not a good idea if the plot will have lots of data.

On the other hand the script can choose not to save animation if the provided data point is too long.
1000 is a good threshold as it can save the animation within 1 minute.

test(plotting): add test for animated plots

…a-plt * 'buda-plt' of https://github.com/YBCS/pyNeuroML: test(plotting): add test for animated plots

* development: fix(nml-channel-analysis): include `ionChannelVShift` Updates to pynml-channelanalysis to allow plotting subgates Updates to xpp running/plotting Indeces -> indices chore: improve annotation tests feat(annotations): make annotations comply with NeuroML/LEMS requirements chore: remove ruff linter pre-commit hook Improved xpp handling Quick & dirty implementation of running xpp files in xppaut & plotting results

YBCS · 2024-05-09T19:29:37Z

tests/plot/test_plot.py

+        ys2 = random.choices(list(range(0, 1500)), k=numpoints)
+
+        generate_plot(
+            [xs, xs],


[xs1, xs2] is it possible that this list contains items with differing lengths. can xs1 and xs2 have different lengths ?

Yes, I think they can, but [y1, y2] must have the same lengths as [x1, x2] so that matplotlib has complete (x,y) co-ordinates for each point.

For animation, one will have to find the values with the longest length and use that to calculate duration and all that then?

"For animation, one will have to find the values with the longest length and use that to calculate duration and all that then?"

Yeah that will be correct as, [plt1, plt2] are individual plots and we only care of the total duration which means the max of the lengths

sanjayankur31 · 2024-05-10T10:27:04Z

Can we also add saving of animations? Looks simple enough?
https://matplotlib.org/stable/users/explain/animations/animations.html#funcanimation

Saving animation is simple. But after some testing with the plots in the codebase I have identified some issues.

Animation is actually very expensive and so it takes lots of time. I have encountered plots with >= 10000 (link) In my machine saving animation plot with data length 1000 can take more almost a minute. Plot is a generic function, it is probably not a good idea if the plot will have lots of data.

On the other hand the script can choose not to save animation if the provided data point is too long. 1000 is a good threshold as it can save the animation within 1 minute.

We can probably just document this in the method docstring and leave it for the user to decide. Often people in research labs have quite good computers, with lots of memory + good cores + gpus and all that. So, in such cases, they may be able to generate animations with more data points. So, it's best to include the functionality and document it. That way users can play around with a few parameters like "duration" and generate what works for them. What do you think?

sanjayankur31

Almost there, I just have a few things I'm unclear on.

sanjayankur31 · 2024-05-10T10:37:10Z

pyneuroml/plot/Plot.py

+                            (save_animation_to))
+                ani.save(
+                    filename=save_animation_to,
+                    writer="pillow",


I wonder if we should make this an argument too, to allow the user to choose different writers? ffmpeg/imagemagick may be quicker than pillow and support more formats:

https://matplotlib.org/stable/users/explain/animations/animations.html

How about making it an argument of the form:

writer={ 'writer name': ["extra args list"] }

with default value:

writer = { "pillow": [] }

?

We should also add this link to the docstring around these arguments:

https://matplotlib.org/stable/users/explain/animations/animations.html#saving-animations

should I include this in the same PR or should it be done in another PR.

I think we can roll this in here too?

sanjayankur31 · 2024-05-10T10:40:53Z

pyneuroml/plot/Plot.py

+            )
+
+            frame_length_threshold = 5000
+            if len(xvalues[0]) < frame_length_threshold and save_animation_to:


This is to limit animations to 5000 frames only? If this depends on the users' setups (how good their hardware etc. is), let's not add this check like this. Maybe we can just add a warning that is thrown if there are more than 5000 points that says "too many data points, this may take a while"? That way the user can still go ahead and use it if they wish. Maybe they'll start it and go eat lunch, and it'll be complete by the time they return (we do this a lot with our simulations :))

I see. I will add a warning and it will ask for user input to continue with the saving or not (possibly showing approx time (minutes) it might take to complete the entire process. )
It will then save or discard based on the user input.
How does that sound ?

I'd skip the user input entirely. Just make it an argument and if it's more than something, just print a warning saying "this could take a while". If it takes too long, the user can always interrupt it and use a smaller value.

(Adding interaction to take user input will make it trickier to do batch analyses, so we want to avoid that as much as possible)

sanjayankur31 · 2024-05-10T10:41:26Z

pyneuroml/plot/Plot.py


    if show_plot_already:
+        if animate:
+            d = 5


I need to test this out more. I thought the idea was that if I set duration to 5000ms, the animation would be 5000ms, but I think you're saying that this is not the case?

yes, that is what is happening with my current implementation.
My calculations could be improved I think and it should be possible to atleast get as close as possible to set duration, let me try again.

Don't worry about getting it exactly right---close enough will do and it can be tweaked later

sanjayankur31

I think that looks good. I'll just test it locally once and then merge ✨

sanjayankur31 · 2024-05-16T16:16:58Z

LGTM, merging. I'll make a few tweaks in my branch and merge to development 👍

sanjayankur31 · 2024-05-16T16:17:58Z

@all-contributors please add @YBCS for code, test

allcontributors · 2024-05-16T16:18:07Z

@sanjayankur31

I've put up a pull request to add @YBCS! 🎉

wip plot animate

bca04af

sanjayankur31 linked an issue Apr 24, 2024 that may be closed by this pull request

Include generation of animated matplotlib plots #279

Closed

sanjayankur31 added the T: enhancement Type: enhancement label Apr 24, 2024

YBCS added 2 commits April 27, 2024 19:55

Merge branch 'development' into buda-plt

f974126

* development: test(annotations): add more assertions feat(annotations): improve docstring feat(annotations): improve for use as embedded annotations chore: to 1.2.13 feat(annotations): extend to allow both miriam and biosimulations styles

chore: add animate hyperparams to plot

c6b45a7

YBCS mentioned this pull request Apr 27, 2024

Include generation of animated matplotlib plots #279

Closed

sanjayankur31 marked this pull request as ready for review April 29, 2024 13:05

sanjayankur31 self-requested a review April 29, 2024 13:09

test(plotting): add test for animated plots

cb5d3f9

sanjayankur31 requested changes May 7, 2024

View reviewed changes

YBCS added 5 commits May 10, 2024 00:20

chore: improve animation hyperparameters and introduce saving animation

04c498d

Merge pull request #1 from NeuroML/feat/plot-animated-test

efe323f

test(plotting): add test for animated plots

Merge branch 'buda-plt' of https://github.com/YBCS/pyNeuroML into bud…

6629a5d

…a-plt * 'buda-plt' of https://github.com/YBCS/pyNeuroML: test(plotting): add test for animated plots

feat: add -save_animation_to param

3437ba4

YBCS commented May 9, 2024

View reviewed changes

YBCS requested a review from sanjayankur31 May 9, 2024 19:32

sanjayankur31 reviewed May 10, 2024

View reviewed changes

YBCS added 2 commits May 14, 2024 22:50

feat: add animate_duration arg for plot

8a8e785

feat: add animate_writer support and tests

fc5be81

YBCS requested a review from sanjayankur31 May 14, 2024 19:28

rename duplicate test name

a632d6c

sanjayankur31 approved these changes May 15, 2024

View reviewed changes

sanjayankur31 changed the base branch from development to feat/test-animate-plots May 16, 2024 16:16

sanjayankur31 merged commit 955b926 into NeuroML:feat/test-animate-plots May 16, 2024

allcontributors bot mentioned this pull request May 16, 2024

docs: add YBCS as a contributor for code, and test #364

Merged

animate matplotlib plots #348

animate matplotlib plots #348

Uh oh!

Conversation

YBCS commented Apr 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanjayankur31 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YBCS commented May 9, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanjayankur31 commented May 10, 2024

Uh oh!

sanjayankur31 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YBCS May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanjayankur31 left a comment

Choose a reason for hiding this comment

Uh oh!

sanjayankur31 commented May 16, 2024

Uh oh!

sanjayankur31 commented May 16, 2024

Uh oh!

allcontributors bot commented May 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

YBCS commented Apr 23, 2024 •

edited

Loading

YBCS May 10, 2024 •

edited

Loading