Method to make multiple animations in one figure #38

johnomotani · 2019-07-17T22:37:35Z

Plot multiple variables at once, like

ds.bout.animate_list([n, V, U])

The variables need to be sliced to 1D+1 or 2D+1 first (can mix 1d and 2d in one call though).

pep8speaks · 2019-07-17T22:37:37Z

Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file xbout/boutdataarray.py:

Line 91:1: W293 blank line contains whitespace
Line 107:90: E501 line too long (90 > 89 characters)

In the file xbout/plotting/plotfuncs.py:

Line 42:24: E251 unexpected spaces around keyword / parameter equals
Line 42:26: E251 unexpected spaces around keyword / parameter equals
Line 213:18: E714 test for object identity should be 'is not'

In the file xbout/plotting/utils.py:

Line 24:49: E127 continuation line over-indented for visual indent

In the file xbout/tests/test_animate.py:

Line 69:54: E127 continuation line over-indented for visual indent
Line 80:59: E127 continuation line over-indented for visual indent
Line 91:54: E127 continuation line over-indented for visual indent
Line 92:53: E127 continuation line over-indented for visual indent
Line 103:54: E127 continuation line over-indented for visual indent
Line 104:53: E127 continuation line over-indented for visual indent
Line 115:54: E127 continuation line over-indented for visual indent
Line 116:53: E127 continuation line over-indented for visual indent
Line 128:54: E127 continuation line over-indented for visual indent
Line 129:53: E127 continuation line over-indented for visual indent
Line 140:54: E127 continuation line over-indented for visual indent
Line 141:53: E127 continuation line over-indented for visual indent
Line 153:58: E127 continuation line over-indented for visual indent
Line 154:57: E127 continuation line over-indented for visual indent
Line 186:57: E127 continuation line over-indented for visual indent
Line 187:56: E127 continuation line over-indented for visual indent
Line 198:54: E127 continuation line over-indented for visual indent
Line 199:53: E127 continuation line over-indented for visual indent
Line 211:54: E127 continuation line over-indented for visual indent
Line 212:54: E127 continuation line over-indented for visual indent
Line 223:54: E127 continuation line over-indented for visual indent
Line 224:53: E127 continuation line over-indented for visual indent
Line 235:54: E127 continuation line over-indented for visual indent
Line 236:54: E127 continuation line over-indented for visual indent
Line 247:54: E127 continuation line over-indented for visual indent
Line 248:53: E127 continuation line over-indented for visual indent
Line 259:54: E127 continuation line over-indented for visual indent
Line 260:53: E127 continuation line over-indented for visual indent
Line 271:54: E127 continuation line over-indented for visual indent
Line 272:53: E127 continuation line over-indented for visual indent
Line 283:54: E127 continuation line over-indented for visual indent
Line 284:53: E127 continuation line over-indented for visual indent

Comment last updated at 2019-12-10 12:37:44 UTC

codecov-io · 2019-07-17T22:44:56Z

Codecov Report

Merging #38 into master will increase coverage by 4.14%.
The diff coverage is 35.8%.

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
+ Coverage   43.47%   47.61%   +4.14%     
==========================================
  Files           8        8              
  Lines         759     1134     +375     
  Branches      144      255     +111     
==========================================
+ Hits          330      540     +210     
- Misses        375      535     +160     
- Partials       54       59       +5

Impacted Files	Coverage Δ
xbout/plotting/utils.py	`6.93% <11.53%> (+2.01%)`	⬆️
xbout/plotting/animate.py	`43.93% <24.61%> (-10.94%)`	⬇️
xbout/plotting/plotfuncs.py	`7.47% <5.71%> (-1.86%)`	⬇️
xbout/boutdataarray.py	`62.96% <52.94%> (-8.47%)`	⬇️
xbout/boutdataset.py	`63.7% <81.66%> (+16.83%)`	⬆️
xbout/load.py	`82.94% <0%> (+5.4%)`	⬆️
xbout/geometries.py	`79.82% <0%> (+7.2%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update befe417...dba0831. Read the comment docs.

johnomotani · 2019-07-18T16:01:50Z

Sorry, I made this PR too soon, should have tried out the new method more first. I've rebased on the current version of 1d-animations now, and fixed a few issues with the original code at the same time.

Previously contained 'animate over <>', but the variable being animated over is shown on the time-slider anyway. Shortening the titles makes things clearer when multiple plots are animated at once.

TomNicholas

This is cool. Have you tried running it with real data? I'm a bit concerned that there might be bugs upstream in animatplot.imshow. Also we should think about how the 2D animations will combine with #34.

xbout/boutdataset.py

TomNicholas · 2019-07-19T09:20:40Z

xbout/plotting/animate.py

@@ -7,7 +7,7 @@


 def animate_imshow(data, animate_over='t', x='x', y='y', animate=True,


It's likely that we shouldn't actually be using imshow, because it's restricted to square images right? Though I originally wrote this for slab cross-sections, then really this should use pcolormesh so that it can handle any geometry? Integrating it with the multi-region plotting method used in #34 will add a bit of complexity (though I think the blocks abstraction will mean it's not too hard).

Think I agree with all of this, although it does need a bit of thinking... Having poloidal plots with geometry included is (very, very) nice to have as an option (maybe even default), but having cartesian as an option too is also useful - for example poloidal plotting of some arbitrary slice is going to be a bit tricky*; also if there are some very small or distorted cells they may be easier to see in a plot that's just rectangular. The blocks abstraction does very much help with all of this, we can add over time functions that create a block in various ways, and code like this that sticks blocks together would only need to add some logic to say which function to call.

It probably would be sensible to use pcolormesh instead of imshow everywhere though, since it's much more flexible.

--

Maybe it would be useful to introduce some 'coordinates' that say which region a point is, e.g. xregion where xregion=0 for x<ixseps1, xregion=1 for ixseps1<=x<ixseps2, xregion=2 for x>=ixseps2 [note ixseps1/ixseps2 include the x-boundary cells, so need to subtract MXG if x does not include the boundary cells]. Hopefully this can be done in such a way that if you do something like n.isel(x=slice(3,7,1) then xregion would get sliced as well. Then we could select regions to join with xregion and a yregion (defined using jyseps*)?

johnomotani · 2019-07-19T15:01:12Z

This is cool. Have you tried running it with real data? I'm a bit concerned that there might be bugs upstream in animatplot.imshow. Also we should think about how the 2D animations will combine with #34.

I have been using it, and it works. Animations sometimes seem to freeze in a jupyter notebook, but I've not worked out how to reproduce yet. It might also be an issue with my setup, I'm currently having problems with matplotlib in ipython, where pyplot.show() is not blocking. Need to spend more time observing when I have problems before trying seriously to debug...

animatplot's Pcolormesh seems to need x- and y-value arrays to be passed explicitly, otherwise the animations looked strange.

Has been updated to use animatplot.Pcolormesh instead of animatplot.Imshow. Also to be compatible with multi-region plotting from #34 and #50.

'norm' option allows any matplotlib.colors.Normalize instance to be passed, for general control of the color-scale. For convenience, also adds a 'logscale' option which can be set to True to create a standard log-scale if vmin and vmax are both positive or both negative, and a symmetric log-scale with a linear threshold of min(abs(vmin),abs(vmax))*1.e-5 if vmin and vmax have opposite signs. If a float is passed for 'logscale' and vmin and vmax have opposite signs then the value of 'logscale' is used instead of the default 1.e-5 to set the linear threshold.

xbout/plotting/animate.py

Had been mixed up in an automatic merge.

rdoyle45 · 2019-12-08T19:28:01Z

Looks good to me. Did a few animations, no issues, worked well.

Allows logscale to be passed per-variable.

TomNicholas

This is a big PR! I've made some comments throughout. I think this is complicated enough that it will take multiple PRs to iron out any bugs and so on

TomNicholas · 2019-12-10T10:02:14Z

xbout/plotting/utils.py

@@ -41,26 +41,29 @@ def _decompose_regions(da):
    j11, j12, j21, j22, ix1, ix2, nin, _, ny, y_boundary_guards = _get_seps(da)
    regions = []

+    x, y = da.dims[-2:]


#68 is merged now so we can replace this with

x = da.attrs['bout_xdim'] y = da.attrs['bout_ydim']

TomNicholas · 2019-12-10T10:03:57Z

xbout/plotting/utils.py

    ystart = 0  # Y index to start the next section
    if j11 >= 0:
        # plot lower inner leg
-        region1 = da[:, ystart:(j11 + 1)]
+        region1 = da.isel(**{y: slice(ystart, (j11 + 1))})


The keyword argument unpacking can be replaced

region1 = da.isel(y=slice(ystart, (j11 + 1)))

TomNicholas · 2019-12-10T10:29:17Z

xbout/boutdataarray.py

+                print("{} data passed has {} dimensions - making poloidal plot with "
+                      "animate_poloidal()".format(variable, str(n_dims)))
+                if x is not None:
+                    kwargs['x'] = x


Should this now default to ds.attrs['bout_xdim']?

I don't think so. Leaving the default as None passes off setting defaults to the called methods, so lets animate_poloidal use (R,Z) and animate_pcolormesh use da.dims.

TomNicholas · 2019-12-10T10:47:29Z

xbout/boutdataset.py

+        try:
+            if len(poloidal_plot) != len(variables):
+                raise ValueError('if poloidal_plot is a sequence, it must have the same '
+                                 'number of elements as "variables"')
+        except TypeError:
+            poloidal_plot = [poloidal_plot] * len(variables)


This will behave weirdly if you get a TypeError that wasn't the one you were expecting. Perhaps instead we should do

if isinstance(poloidal_plot, bool): poloidal_plot = [poloidal_plot] * len(variables) elif len(poloidal_plot) != len(variables) raise ValueError('if poloidal_plot is a sequence, it must have the same ' 'number of elements as "variables"')

This is used 4 times, so could even refactor into a function:

def _expand_arg_list(sequence, arg_name, num_vars): if isinstance(sequence, bool): sequence = [sequence] * num_vars elif len(sequence) != num_vars raise ValueError(f'if {arg_name} is a sequence, it must have the same ' 'number of elements as "variables"') return sequence poloidal_plot = _expand_arg_list(poloidal_plot, 'poloidal_plot', len(variables)) vmin = _expand_arg_list(vmin, 'vmin', len(variables)) vmax = _expand_arg_list(vmax, 'vmax', len(variables)) logscale = _expand_arg_list(logscale, 'logscale', len(variables))

Just as somewhere to note this: to test for a sequence, as suggested on https://stackoverflow.com/questions/2937114/python-check-if-an-object-is-a-sequence, probably best pattern is to use isinstance(variable, collections.Sequence).

TomNicholas · 2019-12-10T10:58:26Z

xbout/boutdataset.py

+            logscale = [logscale] * len(variables)
+
+        blocks = []
+        for this in zip(variables, axes.flatten(), poloidal_plot, vmin, vmax, logscale):


Let's not use this as a variable name 🙏

(subplot?)

TomNicholas · 2019-12-10T11:06:10Z

xbout/plotting/plotfuncs.py

+    gridlines : bool or int, optional
+        If True, draw grid lines on the plot. If an int is passed, it is used as the
+        stride when plotting grid lines (to reduce the number on the plot)


It would have been good to separate these into a different PR but no matter

TomNicholas · 2019-12-10T11:33:52Z

xbout/plotting/plotfuncs.py

+    if gridlines is not None:
+        if gridlines is True:
+            gridlines = (1, 1)
+        if not isinstance(gridlines, collections.Sequence):


collections.sequence seems an odd choice of container class? We just need to store either one or two ints right? Why not just a short list? Or even a dictionary?

gridlines= {'x': 1, 'y':1}

Sorry, this was written quickly. Restricting to int, slice or dict of int or slice is better.

TomNicholas · 2019-12-10T11:34:16Z

xbout/plotting/plotfuncs.py

+        Z_regions = _decompose_regions(da['Z'])
+
+        for R, Z in zip(R_regions, Z_regions):
+            plt.plot(R[::gridlines[0], :].T, Z[::gridlines[0], :].T, color='k', lw=0.1)


Using a dictionary would eliminate the "magic numbers" 0 and 1 here

TomNicholas · 2019-12-10T11:36:19Z

xbout/tests/test_animate.py

+        assert isinstance(animations.blocks[1], Pcolormesh)
+        assert isinstance(animations.blocks[2], Line)
+
+    def test_animate_list_fps(self, create_test_file):


This isn't testing that the fps was actually set. Should do

assert animations.fps == fps

(also to be really nitpicky the animatplot logic is that you have created one animation (singular) which consists of multiple blocks (plural))

johnomotani · 2019-12-10T11:43:49Z

@TomNicholas

This is a big PR!

It is, sorry! Do you want me to try and split it up?

TomNicholas · 2019-12-10T11:46:24Z

It is, sorry! Do you want me to try and split it up?

That might be a good idea - there is at least one clear division: the new arguments to plot2d_wrapper can be their own PR (so gridlines etc. can be discussed elsewhere).

Can now use 'bout_xdim' and 'bout_ydim' metadata to find the renamed x- and y-dimensions.

Also specify in docstring what happens if a dict is passed.

Returned object is a single animatplot 'animation' with multiple blocks, not a list of things.

johnomotani · 2019-12-10T12:38:03Z

Made some updates to address @TomNicholas's review. Will try to split PR now.

johnomotani · 2019-12-10T19:45:43Z

Replaced by #71, #72 and #73.

johnomotani added the enhancement New feature or request label Jul 17, 2019

johnomotani force-pushed the animate_list branch from 9265cee to e9bab98 Compare July 17, 2019 22:40

Method to make multiple animations in one figure

e43fc05

johnomotani force-pushed the animate_list branch from e9bab98 to 5b45807 Compare July 18, 2019 15:59

Shorten titles in animate_imshow and animate_line

39e8fb9

Previously contained 'animate over <>', but the variable being animated over is shown on the time-slider anyway. Shortening the titles makes things clearer when multiple plots are animated at once.

johnomotani force-pushed the animate_list branch from 5b45807 to 39e8fb9 Compare July 18, 2019 16:15

Option to use a color-scale symmetric around 0

5b27452

TomNicholas previously requested changes Jul 19, 2019

View reviewed changes

Raise exception instead of using 'assert'

c96a2da

johnomotani changed the title ~~Method to make multiple animations in one figure~~ WIP: Method to make multiple animations in one figure Aug 10, 2019

johnomotani changed the base branch from 1d-animations to master October 25, 2019 17:16

johnomotani added 15 commits December 4, 2019 12:44

Merge branch '2d-animate-permissive-defaults' into animate_list

bb16d17

Don't pass 'sep_pos' to animate_imshow

13b4c9f

Use pcolormesh instead of imshow for 2d animations

8ddf637

Use animate_pcolormesh in animate_list

34af963

Add support for passing time-varying data to _decompose_regions

408e571

Add option/function to make animated poloidal plots (in R,Z coordinates)

7e3619d

Support for poloidal plots in animate_list

2ab9fea

Fix animate_pcolormesh

2647995

animatplot's Pcolormesh seems to need x- and y-value arrays to be passed explicitly, otherwise the animations looked strange.

Import animate_poloidal to fix poloidal_plots option of animate_list

dabf6a6

Fix colorbar of animate_poloidal when passing in axes object as ax

9eebf99

Allow variable names as well as BoutDataArrays in animate_list variables

25d4836

Fix appending of blocks when using animate_poloidal in animate_list

aff3e8a

Set equal aspect ratio in animate_poloidal

a929445

subplots_adjust option for animate_list

0c4df3e

Arguments to make controls optional in animation methods

9eca9a2

johnomotani added 3 commits December 5, 2019 22:36

Implement logscale option for 2D animation methods

122704b

Document all arguments of animate_list()

625a8e2

rdoyle45 reviewed Dec 8, 2019

View reviewed changes

xbout/plotting/animate.py Outdated Show resolved Hide resolved

rdoyle45 reviewed Dec 8, 2019

View reviewed changes

xbout/plotting/animate.py Outdated Show resolved Hide resolved

Use PillowWriter to save .gif in animate_list

91efd51

rdoyle45 reviewed Dec 8, 2019

View reviewed changes

xbout/plotting/animate.py Outdated Show resolved Hide resolved

Tidy up imports in animate.py

85b0cc6

Had been mixed up in an automatic merge.

johnomotani added 4 commits December 8, 2019 21:45

Allow logscale to be passed as a list to animate_list

9d18e9d

Allows logscale to be passed per-variable.

Fix use of animate_over in animate_line()

26f1e19

Allow poloidal_plot to be passed per variable to animate_list

ee98a70

Tests for animate_list

1bf86d9

TomNicholas requested changes Dec 10, 2019

View reviewed changes

johnomotani added 7 commits December 10, 2019 11:49

Don't rely on order of dims in _decompose_regions()

ef9947d

Can now use 'bout_xdim' and 'bout_ydim' metadata to find the renamed x- and y-dimensions.

Replace redundant argument unpacking in plotting.utils

9f8470f

Simplify checking for sequences in arguments of animate_list

3176809

Rename 'this' -> 'subplot_args'

230d27c

Make specification of 'gridlines' argument to plot2d_wrapper nicer

7ea1c70

Also specify in docstring what happens if a dict is passed.

Rename animations->animation in animate_list tests

3bad703

Returned object is a single animatplot 'animation' with multiple blocks, not a list of things.

Check fps was actually set it test_animate_list_fps

dba0831

This was referenced Dec 10, 2019

Extra options for poloidal plots #71

Merged

animate_list() method to animate multiple plots at once. #73

Merged

johnomotani closed this Dec 10, 2019

johnomotani deleted the animate_list branch December 10, 2019 23:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Method to make multiple animations in one figure #38

Method to make multiple animations in one figure #38

johnomotani commented Jul 17, 2019

pep8speaks commented Jul 17, 2019 •

edited

Loading

codecov-io commented Jul 17, 2019 •

edited

Loading

johnomotani commented Jul 18, 2019

TomNicholas left a comment

TomNicholas Jul 19, 2019

johnomotani Jul 19, 2019

johnomotani commented Jul 19, 2019

rdoyle45 commented Dec 8, 2019

TomNicholas left a comment

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019 •

edited

Loading

johnomotani Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

johnomotani Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

johnomotani Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

TomNicholas Dec 10, 2019

johnomotani commented Dec 10, 2019

TomNicholas commented Dec 10, 2019

johnomotani commented Dec 10, 2019

johnomotani commented Dec 10, 2019

		@@ -7,7 +7,7 @@


		def animate_imshow(data, animate_over='t', x='x', y='y', animate=True,

Method to make multiple animations in one figure #38

Method to make multiple animations in one figure #38

Conversation

johnomotani commented Jul 17, 2019

pep8speaks commented Jul 17, 2019 • edited Loading

Comment last updated at 2019-12-10 12:37:44 UTC

codecov-io commented Jul 17, 2019 • edited Loading

Codecov Report

johnomotani commented Jul 18, 2019

TomNicholas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnomotani commented Jul 19, 2019

rdoyle45 commented Dec 8, 2019

TomNicholas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TomNicholas Dec 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnomotani commented Dec 10, 2019

TomNicholas commented Dec 10, 2019

johnomotani commented Dec 10, 2019

johnomotani commented Dec 10, 2019

pep8speaks commented Jul 17, 2019 •

edited

Loading

codecov-io commented Jul 17, 2019 •

edited

Loading

TomNicholas Dec 10, 2019 •

edited

Loading