Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

heatmap color gradient problem on 32 bit machine with GR #1665

Closed
BoundaryValueProblems opened this issue Aug 16, 2018 · 13 comments · Fixed by #1668
Closed

heatmap color gradient problem on 32 bit machine with GR #1665

BoundaryValueProblems opened this issue Aug 16, 2018 · 13 comments · Fixed by #1668

Comments

@BoundaryValueProblems
Copy link
Contributor

BoundaryValueProblems commented Aug 16, 2018 •
edited

@jheinen , I installed Julia v1.0.0 on my 32 bit linux machine.
The versions are: GR v0.32.2+, Plots vv0.19.2+, Julia v1.0.0.
When I ran heatmap(randn(10,10),ratio=1,c=:viridis), it didn't work and produced the following error. On the other hand, if I do not specify the color gradient scheme and use the default one by just running heatmap(randn(10,10),ratio=1), it works. For 64 bit machines, both of course work.

heatmap(randn(10,10),ratio=1,c=:viridis)
ERROR: MethodError: no method matching length(::ColorGradient)
Closest candidates are:
  length(::Core.SimpleVector) at essentials.jl:571
  length(::Base.MethodList) at reflection.jl:728
  length(::Core.MethodTable) at reflection.jl:802
  ...
Stacktrace:
 [1] _similar_for(::UnitRange{Int32}, ::Type, ::ColorGradient, ::Base.HasLength) at ./array.jl:532
 [2] _collect(::UnitRange{Int32}, ::ColorGradient, ::Base.HasEltype, ::Base.HasLength) at ./array.jl:563
 [3] collect(::ColorGradient) at ./array.jl:557
 [4] broadcastable(::ColorGradient) at ./broadcast.jl:609
 [5] broadcasted(::Function, ::ColorGradient) at ./broadcast.jl:1134
 [6] _update_series_attributes!(::Dict{Symbol,Any}, ::Plots.Plot{Plots.GRBackend}, ::Plots.Subplot{Plots.GRBackend}) at /home/xxx/.julia/dev/Plots/src/args.jl:1575
 [7] _process_seriesrecipe(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}) at /home/xxx/.julia/dev/Plots/src/pipeline.jl:403
 [8] macro expansion at ./logging.jl:320 [inlined]
 [9] _plot!(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}, ::Tuple{Array{Float64,2}}) at /home/xxx/.julia/dev/Plots/src/plot.jl:171
 [10] #plot#134(::Base.Iterators.Pairs{Symbol,Any,Tuple{Symbol,Symbol,Symbol},NamedTuple{(:ratio, :c, :seriestype),Tuple{Int32,Symbol,Symbol}}}, ::Function, ::Array{Float64,2}) at /home/xxx/.julia/dev/Plots/src/plot.jl:57
 [11] (::getfield(Plots, Symbol("#kw##heatmap")))(::NamedTuple{(:ratio, :c),Tuple{Int32,Symbol}}, ::typeof(heatmap), ::Array{Float64,2}) at ./none:0
 [12] top-level scope at none:0

Thanks for your help!
BTW, on 32 bit machine, I still had to change the following part of Plots/src/backends/gr.jl in order for gr backend to use heatmap as we discussed earlier on gitter:

rgba = map(c -> UInt32( round(Int, alpha(c) * 255) << 24 +
                                    round(Int,  blue(c) * 255) << 16 +
                                    round(Int, green(c) * 255) << 8  +
                                    round(Int,   red(c) * 255) ), colors)

Here Int inside the round functions must be replaced by UInt32.
Thanks a lot!
@mkborregaard
Copy link
Member

I cannot reproduce this error, on GR v0.32.2, Plots v0.19.2, PlotUtils v0.5.2, Julia v1.0.0 (metadata versions). Could you free the packages, run ]up and try again?

For the second bit, can we not just change the Int inside the rounding function to UInt?

@anowacki
Copy link
Contributor

anowacki commented Aug 16, 2018 •
edited

I can reproduce this by passing a PlotUtils.ColorGradient to any Plots command (not just heatmap):

julia> using Plots

julia> c = cgrad([:white,:red]);

julia> plot(rand(3), zcolor=rand(3), c=c)
ERROR: MethodError: no method matching length(::ColorGradient)
Closest candidates are:
  length(::Core.SimpleVector) at essentials.jl:571
  length(::Base.MethodList) at reflection.jl:728
  length(::Core.MethodTable) at reflection.jl:802
  ...
Stacktrace:
 [1] _similar_for(::UnitRange{Int64}, ::Type, ::ColorGradient, ::Base.HasLength) at ./array.jl:532
 [2] _collect(::UnitRange{Int64}, ::ColorGradient, ::Base.HasEltype, ::Base.HasLength) at ./array.jl:563
 [3] collect(::ColorGradient) at ./array.jl:557
 [4] broadcastable(::ColorGradient) at ./broadcast.jl:609
 [5] broadcasted(::Function, ::ColorGradient, ::Base.RefValue{Plots.Subplot{Plots.GRBackend}}, ::Int64) at ./broadcast.jl:1139
 [6] _update_series_attributes!(::Dict{Symbol,Any}, ::Plots.Plot{Plots.GRBackend}, ::Plots.Subplot{Plots.GRBackend}) at /Users/nowacki/.julia/packages/Plots/LWBTU/src/args.jl:1566
 [7] _process_seriesrecipe(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}) at /Users/nowacki/.julia/packages/Plots/LWBTU/src/pipeline.jl:403
 [8] macro expansion at ./logging.jl:320 [inlined]
 [9] _plot!(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}, ::Tuple{Array{Float64,1}}) at /Users/nowacki/.julia/packages/Plots/LWBTU/src/plot.jl:171
 [10] #plot#134(::Base.Iterators.Pairs{Symbol,Any,Tuple{Symbol,Symbol},NamedTuple{(:zcolor, :c),Tuple{Array{Float64,1},ColorGradient}}}, ::Function, ::Array{Float64,1}) at /Users/nowacki/.julia/packages/Plots/LWBTU/src/plot.jl:57
 [11] (::getfield(RecipesBase, Symbol("#kw##plot")))(::NamedTuple{(:zcolor, :c),Tuple{Array{Float64,1},ColorGradient}}, ::typeof(plot), ::Array{Float64,1}) at ./none:0
 [12] top-level scope at none:0

Is the answer to define Base.Broadcast.broadcastable(x::ColorGradient) = Ref(x) in PlotUtils, or do the following in Plots/src/args.jl:1565-1566?

    # update series color
    d[:seriescolor] = getSeriesRGBColor.(Ref(d[:seriescolor]), Ref(sp), plotIndex)

I guess this depends on whether ColorGradients are meant to be iterable? I would guess not since there is no iterate method for them. (Another casualty of JuliaLang/julia#18618; see this Discourse post.)

(I ask which is the right approach here just because I don't understand the reason to broadcast the call to getSeriesRBGColor. Happy to introduce a PR either way.)

[Edited to make the example copy-pastable by changing my custom REPL prompt.]

@mkborregaard
Copy link
Member

The reason it is broadcast is because #1467 made it possible to pass a vector of colors to different line segments, requiring the ability to broadcast sometimes, but not always. This creates problems now after julia#18618 .
Most of those broadcast dots can probably be removed again though (I started in #1644 ) and I think that's the right solution, since plot_color is defined for both ColorGradient and Vector{ColorGradient}, we just need to ensure that the functionality introduced in #1467 still works (which is a little harder since our CI pipeline is currently down). I've tried to give a more informative error message JuliaPlots/PlotUtils.jl@ac7e048

@mkborregaard
Copy link
Member

@anowacki a PR would be highly appreciated! The julia version changes are always slightly hectic :-)

anowacki added a commit to anowacki/Plots.jl that referenced this issue Aug 16, 2018
@anowacki
Remove broadcasting of getSeriesRGBColor
ef63c8d 
No longer broadcast over d[:seriescolor] when updating
series attributes.  This fixes JuliaPlots#1665, where in Julia
v1.0 the PlotUtils ColorGradient type is assumed to
be iterable (JuliaLang/julia/#18618).
@anowacki anowacki mentioned this issue Aug 16, 2018
Merged
@BoundaryValueProblems
Copy link
Contributor Author

@mkborregaard , for the argument of that round function, UInt also worked on my 32 bit machine. I would greatly appreciate it if you could change those Int to UInt in backends/gr.jl when you release the next commit.

@BoundaryValueProblems
Copy link
Contributor Author

BoundaryValueProblems commented Aug 17, 2018 •
edited

@mkborregaard and @anowacki , I just updated the packages and rebuilt them. Now I have
GR v0.32.3+; Plots v0.19.2+; PlotUtils v0.5.3 on my 32 bit machine. heatmap(...,c=:viridis) still does not work, but the error messages have been changed as follows:

heatmap(randn(10,10),ratio=1, c=:viridis)
┌ Error: length(::ColorGradient) was called, possibly due to a spuriously broadcast call to a function accepting ColorGradient (e.g. PlotUtils.plot_color). Please open an issue on the library you're using (e.g. Plots)
└ @ PlotUtils ~/.julia/packages/PlotUtils/VLBUW/src/color_gradients.jl:113
ERROR: TypeError: in _similar_for, in typeassert, expected Integer, got Nothing
Stacktrace:
 [1] _similar_for(::UnitRange{Int32}, ::Type, ::ColorGradient, ::Base.HasLength) at ./array.jl:532
 [2] _collect(::UnitRange{Int32}, ::ColorGradient, ::Base.HasEltype, ::Base.HasLength) at ./array.jl:563
 [3] collect(::ColorGradient) at ./array.jl:557
 [4] broadcastable(::ColorGradient) at ./broadcast.jl:609
 [5] broadcasted(::Function, ::ColorGradient) at ./broadcast.jl:1134
 [6] _update_series_attributes!(::Dict{Symbol,Any}, ::Plots.Plot{Plots.GRBackend}, ::Plots.Subplot{Plots.GRBackend}) at /home/xxx/.julia/dev/Plots/src/args.jl:1575
 [7] _process_seriesrecipe(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}) at /home/xxx/.julia/dev/Plots/src/pipeline.jl:403
 [8] macro expansion at ./logging.jl:320 [inlined]
 [9] _plot!(::Plots.Plot{Plots.GRBackend}, ::Dict{Symbol,Any}, ::Tuple{Array{Float64,2}}) at /home/saito/.julia/dev/Plots/src/plot.jl:171
 [10] #plot#134(::Base.Iterators.Pairs{Symbol,Any,Tuple{Symbol,Symbol,Symbol},NamedTuple{(:ratio, :c, :seriestype),Tuple{Int32,Symbol,Symbol}}}, ::Function, ::Array{Float64,2}) at /home/saito/.julia/dev/Plots/src/plot.jl:57
 [11] (::getfield(Plots, Symbol("#kw##heatmap")))(::NamedTuple{(:ratio, :c),Tuple{Int32,Symbol}}, ::typeof(heatmap), ::Array{Float64,2}) at ./none:0
 [12] top-level scope at none:0

@mkborregaard
Copy link
Member

#1670 for the first issue. I'll merge now if you'll try it out offline - I don't think we have CI for 32-bit machines.

@mkborregaard
Copy link
Member

It's not quite fixed yet

@mkborregaard mkborregaard reopened this Aug 17, 2018
@mkborregaard
Copy link
Member

No it is fixed - I was having some trouble keeping track of my branches under Pkg3

@BoundaryValueProblems
Copy link
Contributor Author

BoundaryValueProblems commented Aug 17, 2018 via email

mkborregaard added a commit that referenced this issue Aug 17, 2018
@mkborregaard
Merge pull request #1670 from mkborregaard/master
60b8b00 
Address UInt round problem - see #1665
@BoundaryValueProblems
Copy link
Contributor Author

@mkborregaard , thank you so much! I just pulled the latest master, and now heatmap(randn(10,10),ratio=1,c=:viridis) works on my 32 bit machine!!

@mkborregaard
Copy link
Member

Thank @anowacki ! :-)

@mkborregaard
Copy link
Member

mkborregaard commented Aug 17, 2018 •
edited

So @BoundaryValueProblems about checking out the #1670 PR (which I've already merged and included in an upcoming release but testing on 32-bit would still be very good), you can see at the PR screen that the branch I'm merging from is mkborregaard:master. So you can do ]add https://github.com/mkborregaard/Plots.jl#master to go on that branch and try it out, then ]free Plots to go back to the latest release (and use Julia to track coming releases).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove broadcasting of getSeriesRGBColor anowacki/Plots.jl
3 participants
@anowacki @BoundaryValueProblems @mkborregaard