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.

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

Add contourplot #204

Merged
merged 12 commits into from
Jan 19, 2022
Merged

Add contourplot #204

merged 12 commits into from
Jan 19, 2022

Conversation

t-bltg
Copy link
Member

@t-bltg t-bltg commented Jan 17, 2022

  • add contourplot support (levels, colormap, colorbar controls)
  • add Contour.jl dependency for computing levels using the Marching_squares algorithm
  • add blend flag on all canvases to plot with/without color blend (avoid overlap issues on contour lines)

https://github.com/JuliaGeometry/Contour.jl is a small requirement, since it only depends on StaticArrays.

See examples at JuliaPlots/Plots.jl#4031.

The new example from README.md now looks like this (gaussian profile):
contourplot1

tiny benchmark

using UnicodePlots, BenchmarkTools

main() = begin
  x = -3:.1:3
  y = -7:.1:3
  X = repeat(x', length(y), 1)
  Y = repeat(y, 1, length(x))
  g(x, y, x₀=0, y₀=-2, σx=1, σy=2) = exp(-((x - x₀) / 2σx)^2 - ((y - y₀) / 2σy)^2)
  z = map(g, X, Y)
  @btime heatmap($z)  #   15.090 ms (197665 allocations: 9.45 MiB)
  @btime contourplot($x, $y, $z)  #   2.833 ms (29227 allocations: 1.18 MiB)
end

main()

Needs #203.

@t-bltg
Copy link
Member Author

t-bltg commented Jan 17, 2022

This should be easier to review now, no code style change.

Copy link
Collaborator

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Very nice implementation. I never used Contour before so I don't know how performant it's used in practice. Did you happen to run some real-world examples on this and ensure that it draws within a reasonable time limit?

src/interface/contourplot.jl Outdated Show resolved Hide resolved
src/interface/contourplot.jl Outdated Show resolved Hide resolved
src/interface/contourplot.jl Show resolved Hide resolved
src/interface/contourplot.jl Outdated Show resolved Hide resolved
src/interface/contourplot.jl Outdated Show resolved Hide resolved
@t-bltg
Copy link
Member Author

t-bltg commented Jan 17, 2022

Very nice implementation. I never used Contour before so I don't know how performant it's used in practice. Did you happen to run some real-world examples on this and ensure that it draws within a reasonable time limit?

Well in #204 (comment) I've compared it against heatmap, 7x faster and 8 times less allocs, so I'm not worried there.

I've been using it for a week within my CFD codes (example lid driven cavity in Plots.jl, 256x256), and it is quite fast.

Images:

using BenchmarkTools, UnicodePlots, TestImages

main() = begin
  img = map(Float64, testimage("cam"))  # 512x512
  @btime contourplot($img, levels=1)  #  78.073 ms (779057 allocations: 30.02 MiB)
  @btime heatmap($img)  # 528.955 ms (6558842 allocations: 264.27 MiB)
end

main()

t-bltg and others added 2 commits January 17, 2022 18:58
Co-authored-by: Johnny Chen <johnnychen94@hotmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 17, 2022

Codecov Report

Merging #204 (0ea8571) into master (980d17c) will decrease coverage by 0.08%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
- Coverage   99.65%   99.57%   -0.09%     
==========================================
  Files          23       24       +1     
  Lines        1167     1179      +12     
==========================================
+ Hits         1163     1174      +11     
- Misses          4        5       +1     
Impacted Files Coverage Δ
src/canvas/heatmapcanvas.jl 96.15% <ø> (ø)
src/interface/contourplot.jl 95.23% <95.23%> (ø)
src/canvas/asciicanvas.jl 100.00% <100.00%> (ø)
src/canvas/blockcanvas.jl 100.00% <100.00%> (ø)
src/canvas/braillecanvas.jl 98.07% <100.00%> (ø)
src/canvas/densitycanvas.jl 100.00% <100.00%> (ø)
src/canvas/dotcanvas.jl 100.00% <100.00%> (ø)
src/canvas/lookupcanvas.jl 100.00% <100.00%> (ø)
src/colormaps.jl 100.00% <100.00%> (ø)
src/common.jl 100.00% <100.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 980d17c...0ea8571. Read the comment docs.

@t-bltg
Copy link
Member Author

t-bltg commented Jan 17, 2022

@johnnychen94, quick question about drawing images: what is the convention ?
heatmap does not flip the y axis, and heatmap-ing an image is displayed as upside/down.
I've added the support for contourplot(A) (in addition to contourplot(x, y, A)) which flips the y axis so that the image is not flipped up/down, e.g. contourplot(map(Float64, testimage("cam")), levels=1)

@t-bltg t-bltg force-pushed the contourplot branch 4 times, most recently from 7cccec4 to e3cdcef Compare January 18, 2022 12:22
@t-bltg t-bltg force-pushed the contourplot branch 2 times, most recently from 43e22f1 to 6bdf1d1 Compare January 18, 2022 12:32
@johnnychen94
Copy link
Collaborator

quick question about drawing images: what is the convention ?

From my own experience in image processing, the rotated x-y axis are used so that top-left point is the origin: JuliaImages/juliaimages.github.io#173

Also, because the Julia array is column-major order, the first dimension (x axis) specifies the height instead of the width.

I would try to use the i-th dimension whenever the x/y notation introduces ambiguities.

Copy link
Collaborator

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Clearly, you know more of the plot ecosystem than I do, so my comments here are just double checks from a user's perspective. I'll leave it to you to decide if it's ready to merge.

src/interface/contourplot.jl Show resolved Hide resolved
src/interface/contourplot.jl Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@t-bltg
Copy link
Member Author

t-bltg commented Jan 19, 2022

From my own experience in image processing, the rotated x-y axis are used so that top-left point is the origin: JuliaImages/juliaimages.github.io#173

This means that in julia libraries, input images are stored as regular matrices (origin at North-West, column major), which makes sense.

This is ambiguous for us since contourplot(x, y, A; kwargs...) uses x (West to East) and y (South to North) as in .

(transpose + reverse 1st dim)

PyPlot (row major) uses this convention too: https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.contour.html.

@t-bltg
Copy link
Member Author

t-bltg commented Jan 19, 2022

I think this is pretty clear to me, and from a user point of view:

    contourplot(A; kwargs...)

Contour plot for images (flips the y axis by default)

    Julia matrices (images) |  UnicodePlots
                            | 
           axes(A, 2)       |      
           o------>         |   ^ 
           |                |   |
axes(A, 1) |                | y | 
           |                |   |
           v                |   o------>
                            |       x

And if that doesn't suit someone, they can always use the explicit contourplot(x, y, A; kwargs...).

@t-bltg t-bltg force-pushed the contourplot branch 2 times, most recently from c3ae5a6 to acfb36e Compare January 19, 2022 10:59
@t-bltg t-bltg force-pushed the contourplot branch 5 times, most recently from abe5b2c to eaa2cd7 Compare January 19, 2022 11:31
@t-bltg t-bltg merged commit 891f840 into JuliaPlots:master Jan 19, 2022
@t-bltg t-bltg deleted the contourplot branch January 19, 2022 12:09
@johnnychen94
Copy link
Collaborator

Thank you again for consistently improving this package!

@t-bltg
Copy link
Member Author

t-bltg commented Jan 19, 2022

I released it quickly so that I can move on JuliaPlots/Plots.jl#4031.

I will rework the doc-strings now, and also add a formatting action since as as you reminded me in #199, mixing code style changes and regular changes is not a good idea for reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants