MakieOrg / GeoMakie.jl

Geographical plotting utilities for Makie.jl
https://geo.makie.org
MIT License
167 stars 24 forks source link

too slow at plotting polygons #144

Closed lazarusA closed 3 months ago

lazarusA commented 1 year ago

See the next examples [in a M1 Mac]:

using GeoMakie, GLMakie
using Downloads: download
using GeoMakie.GeoJSON

countries_file = download("https://datahub.io/core/geo-countries/r/countries.geojson")
countries = GeoJSON.read(read(countries_file, String))

function slow()
    cmap = [:white, :black]
    fig = Figure()
    ax = Axis(fig[1,1])
    n = length(countries)
    hm = poly!(ax, countries; color= rand(0:1,n), 
        colormap = cgrad(cmap, categorical=true),
        strokecolor = :black, strokewidth = 0.5,
    )
    Colorbar(fig[1, 2];
        colormap = cgrad(cmap, categorical=true),
        colorrange= (0,1)) # this is also kinda not ideal, the colormap from hm should just work.
    fig
end
@time slow() # second run
25.778192 seconds

and using the GeoAxis

function with_geo()
    cmap = [:white, :black]
    fig = Figure()
    ax = GeoAxis(fig[1,1])
    n = length(countries)
    hm = poly!(ax, countries; color= rand(0:1,n), 
        colormap = cgrad(cmap, categorical=true),
        strokecolor = :black, strokewidth = 0.5,
    )
    Colorbar(fig[1, 2];
        colormap = cgrad(cmap, categorical=true),
        colorrange= (0,1)) # this is also kinda not ideal, the colormap from hm should work.
    fig
end

@time with_geo() # second run
25.894986 seconds
asinghvi17 commented 1 year ago

Yeah, I've had that problem as well. I think CairoMakie might be lowering this to mesh, which causes both the insane file sizes and this issue. Am looking into it.

About the colorbar issue: I think that is a Makie issue with the colorbar layoutable, so it should be its own issue on that repo. Have had the same issue with polys.

SimonDanisch commented 1 year ago

This is with Glmakie... I think @rafaqz is looking into it right now since it likely is a geo interface issue...

asinghvi17 commented 1 year ago

Could be...the issue is that a lot of geo stuff is in multipolygons, and we only specialize for GeometryBasics polygons. ~I'll push a patch up for CairoMakie with a fix on that end.~

Edit: patch is up at https://github.com/MakieOrg/Makie.jl/pull/2590

jkrumbiegel commented 1 year ago

If this is slow in GLMakie, has anyone profiled to see where most time is spent?

rafaqz commented 1 year ago

We need to profile a single multiploygon, this MWE is much too big to profile. I got side tracked trying earlier.

jkrumbiegel commented 1 year ago

Ok for me this alone is terrible (this is the conversion step happening during plotting):

@time multipoly = GeoMakie.to_multipoly.(GeoMakie.geo2basic(countries));
#  41.531221 seconds (36.89 M allocations: 2.336 GiB, 47.79% gc time)

Then creating the figure is actually relatively fast:

function faster(countries)
    cmap = [:white, :black]
    fig = Figure()
    ax = Axis(fig[1,1])
    n = length(countries)
    hm = poly!(ax, countries; color= rand(0:1,n), 
        colormap = cgrad(cmap, categorical=true),
        strokecolor = :black, strokewidth = 0.5,
    )
    Colorbar(fig[1, 2];
        colormap = cgrad(cmap, categorical=true),
        colorrange= (0,1)) # this is also kinda not ideal, the colormap from hm should just work.
    fig
end

@time fig = faster(multipoly);
# 1.110449 seconds (146.72 k allocations: 99.732 MiB, 66.78% gc time)

And displaying is again quite slow:

@time display(fig)
# 7.980616 seconds (17.69 M allocations: 893.942 MiB, 94.68% gc time)

Where exactly it spends its time I'm not sure, because @profview didn't really show anything usable. But if it's 95% gc time, that's not good :D But should be fixable.

jkrumbiegel commented 1 year ago

The problem with this:

@time multipoly = GeoMakie.to_multipoly.(GeoMakie.geo2basic(countries));
#  41.531221 seconds (36.89 M allocations: 2.336 GiB, 47.79% gc time)

boils down to the fact that we have JSON data, and every point we extract gets its own little array because nothing in the pipeline seems to force known-size types like StaticArrays.

rafaqz commented 1 year ago

Totally, I'll look at forcing it to a Tuple length 2 or 3

evetion commented 1 year ago

If we use written out structs in GeoJSON the above conversion GeoMakie.geo2basic(countries) would take:

julia> @time geo2basic(JSON3.read(read(fn), GeoJSON2.GeoJSON));
  0.167426 seconds (1.44 M allocations: 136.101 MiB)
asinghvi17 commented 1 year ago

We added a faster path for vectors of multipolygons in the latest CairoMakie which should alleviate some of this.

rafaqz commented 1 year ago

@evetion also has a PR to make GeoJSON.jl faster: https://github.com/JuliaGeo/GeoJSON.jl/pull/63