Discussion on API to initialize MultiplePlot.

[pfebrer]

Discussion so far on https://github.com/zerothi/sisl/pull/339 (with @zerothi, @tfrederiksen ):

Pol:

grid.plot(represent=["imag", "real"], varying="represent")

This would draw both the imaginary and real contributions in the same plot, while

grid.plot(represent=["imag", "real"], subplots="represent")

would create a subplot for each of them (warning, 3D subplots are not possible yet), and

grid.plot(represent=["imag", "real"], animate="represent")

would create an animation where the first step displays the real part and the second step displays the imaginary part.

So I'd say it is unnecessary to implement it inside GridPlot. It would also make the code more complex. But I'm open to discuss it :)

PD: I don't like the keyword varying at all, if you have a better suggestion it would be great!

Nick:

Ok! :)

What about variables?

Don't know if there are multiple variables, or always 1, in the latter case variable?

Pol:

grid.plot(setting1=[0,1], setting2=[2,3], varying=["setting1", "setting2"])

is allowed. It creates a plot with (setting1, setting2) = (0, 2) and another one where (setting1, setting2) = (1, 3).

Also

grid.plot(varying={"setting1": [0,1], "setting2": [2,3]})

is equivalent.

So, variables? I was looking for some word that makes it clear that they are going to be drawn in the same plot. I find variables better than varying, but still not ideal :) Also it is important that the name does not clash with names that people could give to settings, and in that respect variables seems more dangerous to me. Taking into account that I think I like variable better (as an adjective, not as a noun :smile: ).

Thomas:

Have you considered having a single parameter here to control what kind of representation to be produced, e.g. style : {"single", "multiple", "subplots", "animation"}? To me this seems more intuitive from the user perspective, but perhaps it presents some limitations?

Pol: Yes, but then it is extremely difficult to determine which are the settings that need to be iterated. Unless you have some idea about it that I didn't think of. You can't check for lists, because there might be settings that accept lists as a value.

Nick:

Perhaps it would be useful to use tuples or callable in setting which are then elements in the varying space. This is a bit unintuitive but follows the pandas design where list and tuples have different meanings. It would save an argument.

I think @tfrederiksen has a point, something like:

style={`animate': {"var1": [1, 2, 3], 'style': {"multiple": {"var2": [1, 2, 3]}}}, "multiple": {"var3": [2, 3, 4}}

the dictionary levels would be double nested (no animation within an animation. Whereas there could be as many of the others as needed?

1-keyword here I think would be best... I know it is a bit complicated.. But...

plot(multiple={"var3": [1, 2, 3]}, animate={"var1": [1, 2, 3]})

is already quite verbose, so the chances of this being automated in some way is pretty high.

Alternatively you could create objects which determine how to mix them (they should be hashable, mergeable and iterable), and the contained values could even be grabbed from the __call__ method?

class Animate:
    variable: str
    values: list-like
class Multiple
    variable: str
    values: list-like

var3 = Multiple("var3", [1, 2, 3])
var1 = Animate("var1", [1, 2, 3])
...plot(style=var3 & var1) # meaning both at same time
...plot(style=var3 | var1) # meaning the product of the two combinations

# Or perhaps since they are already kwargs on the call
...plot(var1=Animate([1, 2, 3]), var3=Multiple([1, 2, 3]))

Ok, perhaps this second last idea isn't as great... Just throwing in some ideas.

[pfebrer]

And now I add:

The way I have used it up until now is like:

...plot(setting1=[0,1,2], animate="setting1")

never with the dictionaries, because I agree it's a bit verbose. With that in mind, I think having to write dictionaries with (at least) two levels is not good for usability.

Regarding the Animate and Multiple datatypes, I was exactly thinking the same thing before you posted it :) I think the fact that you need to import them is not optimal, but to me this approach is much cleaner than using nested dictionaries. Also having this datatypes might play nicely with the GUI.

On having nested animations, I think it's not necessary (recommendable) to allow this through a single argument as it starts to get very convoluted and difficult to read. One can always merge plots afterwards. E.g.:

plot1 = geom.plot(setting1=[1,2,3], setting2=1, varying="setting1")
plot2 = geom.plot(setting1=[1,2,3], setting2=2, varying="setting1")

final_plot = plot1.merge(plot2, to="subplots")
#or
final_plot = SubPlots(plots=[plot1, plot2])

[zerothi]

Hmm.

What about having the classes hosted in the Plot class. Something like Plot.defer.Multiple(var1=[1, 2, 3], var2=[2, 3, 4]) etc. It isn't as clean? Or why not just do plot.animate(var1=[0, 1, 2]) ; plot.multiple(var1=[0, 1, 2]) or something similar? Then the method call only has settings as arguments? Alternatively to not have name clashes, plot.animate(variables={'var1': [0, 1, 2]}) etc. I think this last approach would be best to ensure no name clashes?

[pfebrer]

Hmm ok, so the Plot class already has the multiple, subplots and animated class methods. See: https://github.com/zerothi/sisl/blob/fab7c3fe4d467a4f74e13f14cef1b89b1d3ab632/sisl/viz/plotly/plot.py#L344-L592

And actually, what the varying, subplots and animate keywords do is basically to call them. See: https://github.com/zerothi/sisl/blob/fab7c3fe4d467a4f74e13f14cef1b89b1d3ab632/sisl/viz/plotly/plot.py#L99-L119

So, basically I just introduced the keyword arguments because in my view it was more comfortable. Say for example you have a geometry:

geom = sisl.geom.graphene()

And you want to plot it using subplots. If the keywords are available, you can do so like:

geom.plot(var1=[0, 1, 2, 3], subplots="var1")

Otherwise, you need to:

from sisl.viz.plotly import GeometryPlot

GeometryPlot.subplots(var1=[0,1,2,3], fixed={"geometry": geom})

which I think is much worse. Maybe it should be possible to do:

geom.plot.subplots(var=[0,1,2,3])

?

Then there are two problems. First it might get a bit long in case you don't want the default plot:

fdf_sile = sisl.get_sile("file.fdf")
fdf_sile.plot.wavefunction.subplots(i=[0,1,2,3])

(maybe not too bad) And second, I think there are usually more "fixed" settings than "variable" settings, so it might be annoying to write a big dictionary containing all the fixed settings.

[zerothi]

Ok, I see. But how would geom.plot().subplots() work?

I think it should also be clear to the end-user what it does. For instance it is a bit difficult to follow the many options required to fix somethings and vary other things.

1. Using subplots, should probably be done with plot(*fixed options*).subplots(var=[0.1,1]) in this way you define your global plot using plot(...) but redefine them in the subplot stuff?
2. The same approach could be done for animate and multiple

So either there are a couple of more function calls, or the interface for the first calls gets equally long? More or less?

[pfebrer]

Ok, I see. But how would geom.plot().subplots() work?

My proposal was geom.plot.subplots(..., fixed=:::), and it would basically just be a shorthand for GeometryPlot.subplots(..., fixed={"geometry": geom, :::})

Using subplots, should probably be done with plot(fixed options).subplots(var=[0.1,1]) in this way you define your global plot using plot(...) but redefine them in the subplot stuff?

Hmm ok, this is quite nice. So from a already generated plot you generate the subplots. I see a very big problem though. The initial "global" plot would be initialized. This is not possible since it might take a long time and lots of resources to initialize the plot, and that would just be wasted :(

[zerothi]

Would it be possible to lazy-plot stuff? ;)

Also for animations where you wanted to store each animation step but not retain all plots in the memory at the same time?

[pfebrer]

Would it be possible to lazy-plot stuff? ;)

It would definitely be possible, but that would be a global thing, right? Otherwise it would be confusing that sometimes plots are lazily generated and sometimes they are not. I had never thought about it, but from my first thoughts now I don't see clearly whether it's better or not.

Also for animations where you wanted to store each animation step but not retain all plots in the memory at the same time?

Hmm yes totally doable. Specially with https://github.com/zerothi/sisl/pull/312, which totally decouples the rendering part. The issue here would be how crappy the animation looks if the representation is very heavy to compute and/or to transfer.

[zerothi]

How is this issue related to the current state of affairs? Lots have happened?

[pfebrer]

The API for quickly generating multiple plots has disappeared for now :)

The only way to generate a merged plot now is to generate a list of plots and then merge them:

plots = [ ... ]

sisl.viz.merge_plots(*plots)

So now we are completely free to build it from scratch however is best. My idea is to create a fork method in workflows, that can create a view of a workflow with the same nodes until an input is updated, at which point a copy of the affected nodes is created. Similarly to how unix processes work.

That would provide a very easy way of creating plots that share an arbitrary part of the computation. Then it would be a matter of deciding which API would be best for creating the merged plots.

[zerothi]

So should we close this issue?

[pfebrer]

Hmm the discussion for the API will still apply I think