plotter: add HeatMap and Contour implementation #15
Conversation
|
|
||
| // GlyphBoxes implements the GlyphBoxes method | ||
| // of the plot.GlyphBoxer interface. | ||
| func (h *HeatMap) GlyphBoxes(plt *plot.Plot) []plot.GlyphBox { |
There was a problem hiding this comment.
This does not currently do what I want for this type, but is blocked on #14.
kortschak
force-pushed
the
heat
branch
2 times, most recently
from
6c2c053 to
48aaca3
Compare
|
Not sure who's supposed to be reviewing this. We should probably assign people so that it's clear. If it's supposed to be me then I will probably have to wait until this weekend so that I can really look at it. Hope that's OK. |
|
That's fine. I have a partner for it which may go in the same PR - contours. I'm happy to accept any reviewers though. |
|
This is now a WIP. Running @vladimir-ch I have the GridMesh implementation and I can change this to a GridFunc by rearranging the r/c order. PTAL and let me know. |
kortschak
force-pushed
the
heat
branch
6 times, most recently
from
1a55d31 to
a2d4617
Compare
kortschak
force-pushed
the
heat
branch
4 times, most recently
from
c5fd278 to
27c6e60
Compare
|
I've decided the GridFunc approach is nicer and more general. The cost of plugging a matrix in is not onerous and we can provide a helper type if needed. |
|
I did a quick mockup of a way to avoid list.List and code that should be simpler to understand and faster: http://play.golang.org/p/YiLMQ2tdIP. |
|
Thanks. How well does it cope with loop excision? This was the major reason for using lls. I can see that it's just alice tricks, but depending on the data, potentially many.
I think you should be returning point on lines 18 and 19.
|
|
It seems to me contour could be more efficiently just two fields of []point. newContour then becomes `return contour{[]point{l.p1}, []point{l.p2}}`.
|
|
Indeed using points instead of lines makes it much simpler. This is my first time writing this kind of code, I'm not quite sure what you mean by "loop excision" -- although I would expect that it can be incorporated. Current progress so far: https://gist.github.com/egonelbre/5770469d6d7b955a4559. It still has some bugs, but can output something that looks like a contour graph :D. |
|
I had a working version from your play snippet and then I browsed away - I hate mobile devices.
Loop excision is the code at contour.go#212. It is possible to build a path that crosses itself, but if not recognisable as a loop. This is not important at this stage but will be when we want to implement filling.
|
|
Finally have a version where I'm unable to tell the difference visually, although there is some differences... https://gist.github.com/egonelbre/5770469d6d7b955a4559 I'm not sure what to do with the lines that connect to a contour in the middle? See lines 374/378 |
|
I've added a benchmark for contourPaths and relaxed the test for |
kortschak
force-pushed
the
heat
branch
2 times, most recently
from
db68447 to
600c601
Compare
|
@gonum/developers PTAL I think this is ready (except for the volcano example issue which I have skirted by not even mentioning go generate - when/if someone can demonstrate that the data are not subject to the GPL a PR can provide the example complete). |
kortschak
force-pushed
the
heat
branch
12 times, most recently
from
7ae3f1a to
387b80a
Compare
|
|
||
| cases = [3][3][3]int{ | ||
| {{0, 0, 8}, {0, 2, 5}, {7, 6, 9}}, | ||
| {{0, 3, 4}, {1, 0, 1}, {4, 3, 0}}, |
There was a problem hiding this comment.
The middle point of this is "3" in the conrec.c algorithm, but is 0 here. Is that intentional?
There was a problem hiding this comment.
Yes. I wrote to Paul about that, but haven't had a reply.
I believe the 3 in that position is incorrect when considering the commentary in the paper.
axiom: When we have a section of the grid where all the Z values are equal, and equal to a contour height we would expect to have no internal segments to draw.
This is covered by case g) in his description of the algorithm (a triangle with three vertices the lie on the contour level). He says "... case g above has no really satisfactory solution and fortunately will occur rarely with real arithmetic." and then goes on to show the following:
which shows case g) in the set where no edge is drawn.
However, in the iteration over sh at conrec.c +44 (conrec.go +74), a triangle with all vertices on the plane is given sh = {0,0,0,0,0} and then when the switch at conrec.c +93 (conrec.go +128) happens, castab/cases resolves that to case 3 for all values of m. This is fixed by replacing castab/cases[1][1][1] with 0.
|
I don't fully comprehend a lot of the contour code, but otherwise LGTM |
|
Are there sections that are a concern in particular wrt comprehension. I would like to make this as clear as possible. If comments in any sections would help, I'll add them. |
|
@egonelbre would you please take a quick look at this? |
|
I took a quick look and based on that quick look LGTM. Basically, didn't notice anything sticking out. |
kortschak
force-pushed
the
heat
branch
2 times, most recently
from
0ad5f75 to
82c9a8e
Compare
Replace list-based approach with bi-directional slice-based approach suggested by Egon Elbre. Also add complete graph-based analysis for contour path reconstruction with optimisation checks for simple or no-work cases. Comparison between list and current path reconstruction: benchmark old ns/op new ns/op delta BenchmarkComplexContour0 12813961 6131752 -52.15% BenchmarkComplexContour1 23735387 10794846 -54.52% BenchmarkComplexContour2 36366033 17146897 -52.85% BenchmarkComplexContour4 57171163 29761464 -47.94% BenchmarkComplexContour8 107407179 58964940 -45.10% BenchmarkComplexContour16 181637216 112427754 -38.10% BenchmarkComplexContour32 293213592 175411602 -40.18% A naive plotting option is retained to allow debugging.
|
Thank you everyone. |
|
Thank you for putting that together! Contour maps are very useful (now I feel worse about the lack of a quadrature package). |
No description provided.