Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Heatmaps with irregular grids #137

Merged
merged 11 commits into from
May 10, 2021
Merged

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Nov 1, 2020

See MakieOrg/Makie.jl#675 and JuliaPlots/CairoMakie.jl#111

Currently works with irregular vectors such as in the example from the issue. Still working on getting all the other inputs to work.

Screenshot from 2020-11-01 18-44-10

@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 2, 2020

Tests passed locally.

@ffreyer ffreyer changed the title Heatmaps with irregular grids (WIP) Heatmaps with irregular grids Nov 2, 2020
@ffreyer ffreyer changed the title Heatmaps with irregular grids Heatmaps with irregular grids (WIP) Nov 3, 2020
@jkrumbiegel
Copy link
Member

I never really took note of this, we should try to get this in! But I want to tackle the problem with centering at the same time, so that bins are centered on the values in the x and y ranges. I think for that, changes in AbstractPlotting regarding the heatmap conversion pipeline might be necessary?

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 30, 2021

I think that's just a question of where we want to deal with that. Do we want backends to assume every heat map is centered? Should every heatmap be centered, even heatmap(0..10, 0..10, rand(10, 10))?

It would be good if Intervals reach the backend though, since these can be handled more optimally. (Pretty sure I removed those optimizations in this pr though. I recall having some issues getting them to work with the changes I made)

To avoid sitting on this for another 5 months, maybe we should just say a heatmap in always centered for now, and work out the details later. It should be fine to handle that in the backends.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 30, 2021

These all work as expected: (still without centering)

heatmap(rand(10, 10))
heatmap(1..2, 1..3, rand(10, 10))
heatmap(1:10, 1:10, rand(10, 10))
heatmap(1:10, [1,3,4,5,6,8,10,14,15,20], rand(10, 10))
heatmap([1,2,3,4,5,10,11,12,13,20], [1,3,4,5,6,8,10,14,15,20], rand(10, 10))

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 30, 2021

heatmap(1..3, 1..3, rand(3, 3))
heatmap(1:3, 1:3, rand(3, 3))

results in
Screenshot from 2021-03-30 22-40-33

heatmap([1, 3, 4], [1, 2, 4], rand(3, 3))

results in
Screenshot from 2021-03-30 22-41-39

And

heatmap(rand(3, 3))

results in
Screenshot from 2021-03-30 22-42-49
which isn't right anymore. The convert_arguments in AbstractPlotting needs to have a method for Heatmap using 1..n Intervals to fix this. We also need to adjust limits/bounding boxes

@ffreyer ffreyer changed the title Heatmaps with irregular grids (WIP) Heatmaps with irregular grids Mar 30, 2021
@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 30, 2021

For shifting I copied what I had in MakieOrg/Makie.jl#675.

@jkrumbiegel
Copy link
Member

jkrumbiegel commented Mar 31, 2021

I had proposed earlier to use an Edges type for this, so that we could convert ranges, vectors, intervals... to it and have an unambiguous representation in the backend. Because the centers don't immediately specify where the boundaries would end up. And then people could also immediately feed such Edges to the plotting function if they have that data instead of cell centers. What do you think, could you add that to your solution here (on the AbstractPlotting layer)?

@ffreyer
Copy link
Collaborator Author

ffreyer commented Mar 31, 2021

heatmap(rand(3, 3)) # centered on 1 to centered on 3 in both dimensions
heatmap(1:3, 1:3, rand(3, 3)) # same
heatmap(1..3, 1..3, rand(3, 3)) # same
heatmap([1, 2, 4], [1, 3, 5], rand(3, 3)) # unequal cell sizes, still giving the centers

heatmap(Edges(1..3), Edges(1..3), rand(3, 3)) # now the edges go from 1 to 3, not the centers
heatmap(Edges(1..3, 1..3), rand(3, 3)) # same, maybe more convenient with only one `Edges`
heatmap(Edges([1, 2, 4, 5], [2, 5, 6, 7]), rand(3, 3)) # unequal cell sizes

This seems doable, but I'm not sure what I'd have to do in AbstractPlotting to make it work. In GLMakie it would be nice to have separate Edges (one for xs, one for ys) and to have Intervals/ranges alongside Vectors for optimizations. If ranges are always just (start, step, stop) it would be fine to convert them to intervals or vice versa.

Comment on lines 326 to 360
gl_attributes[:position_x] = if to_value(x[1]) isa Vector
map(x[1]) do v
# Equivalent to
# mids = 0.5 .* (v[1:end-1] .+ v[2:end])
# borders = [2v[1] - mids[1]; mids; 2v[end] - mids[end]]
borders = [0.5 * (v[max(1, i)] + v[min(end, i+1)]) for i in 0:length(v)]
borders[1] = 2borders[1] - borders[2]
borders[end] = 2borders[end] - borders[end-1]
Texture(el32convert(borders), minfilter = :nearest)
end
else
map(x[1]) do _x
min, max = to_range(_x)
N = size(to_value(x[3]), 1)
halfstep = 0.5 * (max - min) / (N-1)
a = collect(range(min-halfstep, max+halfstep, length=N+1))
Texture(el32convert(a); minfilter=:nearest)
end
end
gl_attributes[:position_y] = if to_value(x[2]) isa Vector
map(x[2]) do v
borders = [0.5 * (v[max(1, i)] + v[min(end, i+1)]) for i in 0:length(v)]
borders[1] = 2borders[1] - borders[2]
borders[end] = 2borders[end] - borders[end-1]
Texture(el32convert(borders), minfilter = :nearest)
end
else
map(x[2]) do _x
min, max = to_range(_x)
N = size(to_value(x[3]), 2)
halfstep = 0.5 * (max - min) / (N-1)
a = collect(range(min-halfstep, max+halfstep, length=N+1))
Texture(el32convert(a); minfilter=:nearest)
end
end
Copy link
Collaborator Author

@ffreyer ffreyer Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Edges{Vector} this would turn into just

gl_attributess[:position_x] = map(e -> Texture(el32convert(e.data), minfilter = :nearest), x[1])
gl_attributess[:position_y] = map(e -> Texture(el32convert(e.data), minfilter = :nearest), x[2])

Edges{<:AbstractRange} and Edges{<:Interval} could use some implementation based on Grid like surface, but I think there are also changes to the shader that need to happen. So maybe we just convert to Vector here for now, and optimize this later. So

gl_attributess[:position_x] = map(x[1]) do e
    if e.data isa Vector
        Texture(el32convert(e.data), minfilter = :nearest)
    else
        min, max = to_range(e.data)
        N = size(to_value(x[3], 1)
        Texture(el32convert(collect(range(min, max, length=N))), minfilter = :nearest)
end
...

@ffreyer
Copy link
Collaborator Author

ffreyer commented May 4, 2021

I adjusted things to work with JuliaPlots/AbstractPlotting.jl#727. Now heatmap assumes vector inputs for x and y, which means it won't work without 727.

@ffreyer
Copy link
Collaborator Author

ffreyer commented May 5, 2021

I need to recover the Interval methods because they still make it through

@ffreyer
Copy link
Collaborator Author

ffreyer commented May 6, 2021

heatmap(1..3, 1..2, rand(10, 10)) works again

@SimonDanisch SimonDanisch merged commit 61fd113 into JuliaPlots:master May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants