JuliaML / MLPlots.jl

Plotting recipes for statistics and machine learning using Plots.jl
Other
24 stars 5 forks source link

Add Plots for numeric ValueHistories #5

Closed Evizero closed 8 years ago

tbreloff commented 8 years ago

Did you review whether these can be implemented as PlotRecipes? You really shouldn't have to ever implement a plot or plot! method, as then you bypass lots of built-in functionality. Worst case, you may want to implement a Plots.createKWargsList method (I should really change that ugly name). I can help with either implementation, check out this recipe for an example of how you might attempt this.

Evizero commented 8 years ago

Wouldn't that require everything I pass to plot to be a subtype of PlotRecipe (which kinda defies the purpose)? Or is is duck-typing in the sense that I only really have to implement getRecipeXY(mycustomobject) and getRecipeArgs(mycustomobject)

The goal I have is to be able to use plot(mycustomobject) with the typical kw-args or specialized further positional args. Do PlotRecipes allow me to do that?

tbreloff commented 8 years ago

Let me think through this a little more, and I'll come back with what I would do.

Evizero commented 8 years ago

try it on the loss plots, which are a little more complicated as I would like to support something along the line of plot(loss) (which uses a default range) and plot(loss, -2, 2)

tbreloff commented 8 years ago

Ok I'll put together an IJulia notebook, and I might make some code changes in Plots to support it better.

On Tue, Nov 24, 2015 at 10:06 AM, Christof Stocker <notifications@github.com

wrote:

try it on the loss plots, which are a little more complicated as I would like to support something along the line of plot(loss) (which uses a default range) and plot(loss, -2, 2)

— Reply to this email directly or view it on GitHub https://github.com/tbreloff/MLPlots.jl/pull/5#issuecomment-159295125.

tbreloff commented 8 years ago

Here's a possibility (you'll need to checkout dev to try it out):

tmp

tbreloff commented 8 years ago

Also you could make a small adjustment for the subplot case, which sets the titles instead of the labels if you have 1 loss per plot:

function Plots._apply_recipe{L<:Loss}(d::Dict, losses::AbstractVector{L}; kw...)
    ...
    if get(d, :n, 0) == length(losses)
        d[:legend] = false
        d[:title] = d[:label]
    end
    ...
end
Evizero commented 8 years ago

That seems cool! One question. How would I implement the functionality that allows plot(myloss, -3, 3) ?

I would guess: Plots._apply_recipe{L<:Loss}(d::Dict, losses::AbstractVector{L}, x1::Real, x2::Real; kw...) ?

tbreloff commented 8 years ago

Sure that would work. And any variation of it, as long as the types aren't too generic. Just keep in mind... if you did something like:

Plots._apply_recipe(d::Dict, x::Vector{Float64}; kw...) = ...

then it would seriously break things since all float vectors would now pass through your recipe code. For custom types this shouldn't be an issue.

Evizero commented 8 years ago

Ok nice. I'll give it a shot. One concern though. Will d[:legend] = false just overwrite the defaults, or will it force the values to be this way no matter what the user specifies using the KWs. Because what we really want is to specify custom default values for title etc which the user can overwrite using the usual KWs

tbreloff commented 8 years ago

That's up to you... the kw... is what the user put there, and d should be the same as kw, but after some preprocessing. You could do something like d[:legend] = get(d, :legend, false), which I think will do what you want. This should use the existing value, or add your default value if the user didn't set it.

Evizero commented 8 years ago

new problem. Let's take an SVM classification plot for example where I want to do plot(trained_svm). I would need to plot some contours, then some points, then the support vectors, and then maybe a line. How would I approach that without overloading plot?

svm

tbreloff commented 8 years ago

Can you post the code (or a gist) with a full example? Then I can help create the matching function.

But generally, you can set arg/d with the same tricks you would normally. Notice that in the 2nd _apply_recipe above, I'm creating multiple series all at once by:

So you can get creative... d[:linetype] = [:contour :scatter :path], etc

Evizero commented 8 years ago

so I am trying to change to this approach and I am getting a weird error

function Plots._apply_recipe{I,V<:Real}(d::Dict, h::VectorUnivalueHistory{I,V}; kw...)
    d[:marker] = get(d, :marker, (:ellipse, 1.5))
    d[:legend] = get(d, :legend, false)
    get(h) # Tuple{Array{Int64,1},Array{Float64,1}}
end
ERROR: Unknown key: marker
 in default at /home/csto/.julia/v0.4/Plots/src/args.jl:324
 in warnOnUnsupportedArgs at /home/csto/.julia/v0.4/Plots/src/args.jl:579
 in plot! at /home/csto/.julia/v0.4/Plots/src/plot.jl:87
 in plot at /home/csto/.julia/v0.4/Plots/src/plot.jl:56
Evizero commented 8 years ago

Are you sure you want to go this direction of having this custom way to define new plots? It feels really strange. overloading plot feels much more intuitive and flexible for complicated plots which one could then build up incrementally

tbreloff commented 8 years ago

ERROR: Unknown key: marker

The _apply_recipe method is called after argument preprocessing, which is where the "magic args" like :marker are handled. So you would need to be explicit in setting the individual args:

d[:markershape] = :ellipse
d[:markersize] = 1.5
...

Are you sure you want to go this direction of having this custom way to define new plots?

I'm pretty sure that it's a bad idea to overload plot, plot!, and all the variations of subplot/subplot! (probably 5-6 methods per recipe) just so you can make a familiar call to plot in the function. I think the _apply_recipe method is very straightforward once you understand the rules.

Also remember that your average user won't be creating recipes (or at least, not right away) so it can be slightly less intuitive while saving tons of repeated (and error prone) code.

Evizero commented 8 years ago

So I gave it a shot. Let me know what you think.

Any ideas how I could infer n for the subplots based on my history? So that the user doesn't need to know how many subplots are needed.

subplot(history)
ERROR: You must specify either layout or n when creating a subplot: Dict{Any,Any}()
 in subplot at /home/csto/.julia/v0.4/Plots/src/subplot.jl:166
tbreloff commented 8 years ago

Pull the latest dev branch, and you'll be able to do something like this:

function Plots._apply_recipe{L<:Loss}(d::Dict, losses::AbstractVector{L}; issubplot=false, kw...)
    d[:xlabel] = map(_loss_xlabel, losses)'
    d[:ylabel] = "L(y, h(x))"
    d[:label] = map(string, losses)'
    if issubplot
        n = get!(d, :n, length(losses))
        if n == length(losses)
            d[:legend] = false
            d[:title] = d[:label]
        end
    end
    map(value_fun, losses), -2, 2
end

Note: the issubplot arg in the signature is optional... only include it if you need to.

If the user sets n, it uses that. Otherwise I'm setting it to the length of losses.

tbreloff commented 8 years ago

I'm happy to merge what you have so far, and we can fix/fine-tune what we need to later.

Also with regards to testing... I'm working on releasing a new package https://github.com/tbreloff/VisualRegressionTests.jl which will contain a generalized version of the regression testing that I'm doing in Plots currently. When that's ready to go we should build some examples which incorporate the plots we're building here, and run them as part of the travis testing. I'll get the framework setup... then you can add the tests for recipes that you add.

tbreloff commented 8 years ago

A quick side note... thanks to this PR, I changed the default _apply_recipe and now we don't have to specify n all the time. This works now:

subplot(rand(10,4))

Before it would error because of the missing n=4. Thanks!

Evizero commented 8 years ago

Nice. Don't merge just yet. I would like to add the proposed changes and also do a little monkey testing.

Evizero commented 8 years ago
get!(d, :markershape, :ellipse)
get!(d, :markersize, 1.5)

this works for plot but not for subplot

tbreloff commented 8 years ago

Can you open an issue? Sounds like a bug, but I'm not at my computer.

On Nov 24, 2015, at 5:59 PM, Christof Stocker notifications@github.com wrote:

get!(d, :markershape, :ellipse) get!(d, :markersize, 1.5) this works for plot but not for subplot

— Reply to this email directly or view it on GitHub.

Evizero commented 8 years ago

I moved the files into sub-folders and implemented your suggestions. I will adapt the loss function plots in a separate PR. I'm ok with merging this if you are.

concerning issue: yes sure, will do