Simple interface for plot field for both time and frequency

[arturgower]

I think the following should work:

• plot(SimulationResult) should return several lines plots for each x
• plot(SimulationResult, linetype=:contour) should attempt a contour plot. This should work for both TimeSimulationResult and FrequencySimulationResult. Note the function run(Simulation,shape) can return both TimeSimulationResult and FrequencySimulationResult, which can then be plotted
• plot(FrequencySimulation, ω) should generate a contour plot.

There might be a dispatch issue for the first two, as I'm struggling to get plot recipes to do this...

[arturgower]

Have a look at the first function in plots.jl. plot(SimulationResult) works, but plot(SimulationResult, linetype=:contour) and plot(SimulationResult, linetype=:field) do not...

[jondea]

Plots is quite complicated because linetype can be define in several ways, in commit 7a66332 I use
plotattributes[:xlims] to access xlims as defined in any other way. This may help to make the function more robust.

Another thing to think about is, should we allow surface type plots? These can accept vectors of x, y and z with no real structure and infer a triangular interpolation. Using something like this would mean we wouldn't have to assume that the user provided x vector is structured nicely. It should also be possible to do this for a contour plot, but I can't seem to do it.

Here's a quick example to show surface plots with unstructured sample points:

rx = 2.*(rand(1000).-0.5)
ry = 2.*(rand(1000).-0.5)
surface(rx,ry,rx.^2.+ry.^2)

[jondea]

A simulation result can have any number of angular frequencies or positions. What if we pretty much ignore linetype and have plot(simresult, x_vec, ω) where the user can put

plot(simresult, :, ω) # Plot all positions for one angular frequency, i.e. contour
plot(simresult, x, :) # Plot one position for all angular frequencies, i.e. line

We can then detect : with something like (:) === Colon(). User could also pass in vector of Floats, vector of SVectors or vector of indices. If both were vectors then I'm not sure what we would do.

PS: are ω and x the wrong way around everywhere? Does it make sense to have angular frequency or position first?

[arturgower]

I think having a plot, like surface, that takes any x is awesome. But really it needs to be a 2D plot, for quality and to show particles/sources. Compare the below surface and contour:

Do you think that perhaps plot(sim, x, ω) would be better for surface plot? That way the user can pass any x and ω (and not pass the indices, which is a bit awkward). I think that would be nicer.

To plot FrequecySimulationResult and TimeSimulationResult, I'm not sure. I'm thinking that plot(simresult, x_indices, ω_indices) is a bit awkward. For a user that knows the x_indices they want, we can provide a function like simres2 = deepcopy(simres; x_indices = ...) that creates a copy with only those x_indices, then the user can just plot(simres2, ω), where plot finds the closest simres.ω to ω and plots that. Also as an optional argument plot(simres, ω; x_indices = ..) would be good too!

For now, how about we implement plot(simres, ω; seriestype=surface, x_indices = : ) and plot(simres, ω; seriestype=contour, x_indices = : ). If the user provides the wrong format for seriestype=contour we can throw an error.

Btw, was there a reason you specialised the plot functions to FrequecySimulationResult? I've converted it back to SimulationResult, which needed only a small tweak.

[arturgower]

Here is my suggestion. I know you don't love keywords! But for me this the most useful and intuitive option:

• plot(simres; kws..) shows line plots, where kws can include x_indices =.., x=Vector{AbstractVector} and ω_indices = ....
• plot(simres, ω; kws..) is either a surface (default) or contour plot, depending on kws. Advanced user can further restrict x_indices = .., but it would be much nicer to pass, say, bounds = Rectangle(...). Hard to imagine using x_indices = ...

What do you think?

[arturgower]

And sorry it currently does not work and I couldn't find why! I have to run off and enjoy Kyoto now.

[jondea]

Contour and surface are entirely a matter of taste, and we should really be able to do both whatever the user gives us. I just can't seem to get contour to accept unstructured data or, equally, extract the contour plot embedded in surface. I think without significant effort, for now we are limited by what plots can cope with. Also, surface looks odd from above, it's only really useful from an angle.

The reason I specialised the function to frequencies was because getfield is brittle and unclear, also any keyword arguments or text called ω don't make sense in the context of time. src/plot/plot.jl:7 shows the kind of error which is very easy to make and hard to spot. It's true that there are probably ways around all of these problems. I tend to err on the side of defensive programming, but in this case I don't have strong feelings.

Absolutely, indices are not the most natural way of interacting for a user, I intended to implement a wrapper function much like you have. Although I intended to make more use of dispatch rather than keywords, but I quite like how you've done it. I'm not generally a big fan of keywords if the same thing can be achieved without them, but for high level stuff like plotting they can make a lot of sense.

[arturgower]

Yes, just adding some wrappers for your index function would be nice. I just didn't think of it at the time!
I fully support writing more robust functions that don't use getfield too.

The cause of the error was the weirdest thing:

@recipe function myplot(x::Vector{Int}; t::AbstractFloat = 2.)
   (x,t*x)
end

but then

plot([1:2]) 
>ERROR: MethodError: Cannot `convert` an object of type Expr to an object of type Symbol

but if you don't type declare the keyword t, such as the below, then there would be no error.

@recipe function myplot(x::Vector{Int}, y::Int; t = 2)
   (x,t*x)
end

that is, plot([1,2],2) works.