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 TriMesh element #2143

Merged
merged 28 commits into from Dec 14, 2017

Conversation

Projects
None yet
4 participants
@philippjfr
Member

philippjfr commented Nov 22, 2017

Triangle meshes are a common form of data when working with complex polygons and are frequently employed in environmental modeling. The most common way to compute such a mesh is using Delaunay triangulation and the data is usually represented as two data structures:

  1. The simplices representing each triangle, usually these are node indices.
  2. The nodes or points representing each corner of a triangle.

This representation is in fact simply a specific type of graph and closely follow the data structures we already use for graphs, where the simplices represent the abstract connectivity of the mesh and the nodes represent the positions. It was therefore trivial to write a TriMesh element which simply reuses the same data structures, organization, and plotting code as existing graph elements.

Here is a simple example:

import param
import numpy as np
import holoviews as hv
from scipy.spatial import Delaunay

from holoviews.element.graphs import Graph, EdgePaths

hv.extension('bokeh')

n_verts = 1000
pts = np.random.randint(1, n_verts, (n_verts, 2))
tris = Delaunay(pts)

hv.TriMesh((tris.simplices, tris.points))

bokeh_plot

This approach is reasonably fast for small meshes (~1 second/5000 triangles) and once you start plotting more than ~10k triangles you will want to use datashader anyway. It is also flexible enough to associate additional values both with the simplices and with the nodes.

Once pyviz/datashader#525 is merged I will get on with allowing datashader operation such as aggregate to operate on the TriMesh element.

  • Reference gallery entries
  • Unit tests

@philippjfr philippjfr added the feature label Nov 22, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Nov 23, 2017

If you enable WebGL rendering in bokeh you can actually scale up to some pretty large meshes, here is an example of selecting on 40485 triangles:

screen shot 2017-11-23 at 1 55 00 am

screen shot 2017-11-23 at 2 07 51 am

This was referenced Nov 23, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Nov 24, 2017

I've now allowed filling the triangles by a value, so here's the example above this time with filled triangles:

screen shot 2017-11-24 at 4 47 54 pm

I'd now consider this PR ready for review, I've added unit tests and reference notebooks. The datashading portion will follow in another PR once it's been merged into datashader.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Nov 24, 2017

Yay, we even gained coverage.

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 2, 2017

I am trying this out since I work a lot with triangulations, currently, I create plots like:
plot
(an Overlay of Polygon and Image)

Using this branch I am trying to get something similar, but it's not getting better:

%%output size=200
%%opts TriMesh (node_size=0 edge_line_width=0.1 edge_nonselection_alpha=0.01)
plot * hv.TriMesh((tris.simplices, points))

mr

I am unable to set the alpha and linewidth, am I not understanding it, or is this a bug?

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 2, 2017

Another question, the hv.Image I used in the Overlays above are created from an interpolation that uses the scipy.spatial.Delaunay object, but looking at your example plot it seems to be possible to fill the triangles directly with the values at the vertices, but I can't see how exactly. Could you please give a simple example?

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 3, 2017

Looks like you're using the matplotlib backend but specifying bokeh options, try changing that to edge_linewidth and edge_alpha.

but looking at your example plot it seems to be possible to fill the triangles directly with the values at the vertices, but I can't see how exactly.

Have a look at the example notebook, the last example demonstrates it. Going to be pushing a small fix for that shortly.

@philippjfr philippjfr requested review from jlstevens and jbednar Dec 8, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 8, 2017

@jlstevens @jbednar Requesting review.

@stonebig stonebig referenced this pull request Dec 8, 2017

Closed

release 2018-01 follow-up #574

@jbednar

jbednar approved these changes Dec 8, 2017

Looks great!

Element or overlay of Elements into an hv.Image or an overlay of
hv.Images by rasterizing it, which provides a fixed-sized
Element or overlay of Elements into an hImage or an overlay of
.Images by rasterizing it, which provides a fixed-sized

This comment has been minimized.

@jbednar

jbednar Dec 8, 2017

Member

What are hImage and .Image ?

return Image(agg, **params)
class rasterize(trimesh_rasterize):

This comment has been minimized.

@jbednar

jbednar Dec 8, 2017

Member

How can rasterize be derived from trimesh_rasterize? That seems like a broken type hierarchy, as it does not satisfy "is a" ("rasterize" is not a kind of "trimesh_rasterize").

This comment has been minimized.

@philippjfr

philippjfr Dec 8, 2017

Member

True, it was easiest to inherit all the methods and parameters but a cleaner class hierarchy is worth the extra handling.

(not isinstance(x, Image) or x in imgs))
element = element.map(dsrasterize, predicate)
return element

This comment has been minimized.

@jbednar

jbednar Dec 8, 2017

Member

Does this not yet handle regridding?

This comment has been minimized.

@philippjfr

philippjfr Dec 8, 2017

Member

I was going to handle that in another PR once datashader has made the API more consistent.

This comment has been minimized.

@jbednar

jbednar Dec 8, 2017

Member

Ok. And that's on my plate, but after this conference prep...

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 8, 2017

Great work! 👍

How can I use the interpolation here?

How do I go from:

%%opts TriMesh [filled=True edge_color_index='z'] (cmap='viridis' node_alpha=0 edge_line_alpha=0.4)
def plot_TriMesh(learner):
    ip = learner.ip()
    simplices = ip.tri.simplices
    nodes = ip.tri.points
    z = ip.values[simplices].mean(axis=1)
    return hv.TriMesh((np.column_stack([simplices, z]), nodes), vdims='z')
plot_TriMesh(learner)

trimesh

to

%%opts Image (cmap='viridis')
%%opts Contours (color='k')
learner.plot(triangles_alpha=0.4)

adaptive

I would like to know because:

%timeit plot_TriMesh(learner)
2.71 ms ± 309 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

%timeit learner.plot(triangles_alpha=0.4)
216 ms ± 13.9 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

EDIT
the timing is not such an issue anymore, implemented an overlay of an Image and TriMesh

# new
%timeit learner.plot(triangles_alpha=0.4)
11.7 ms ± 803 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# old
%timeit learner.plot(triangles_alpha=0.4)
317 ms ± 17.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

however learner.plot() now fails when there is no data yet.

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 8, 2017

For the same reason hv.Image([]) is useful, the same would go for hv.TriMesh.

Would it be easy to make this work with hv.TriMesh(([], []))or something?

My data is calculated and plotted live, and in be beginning there might not be data to plot yet.

Then I could write beautiful things like:

if self.data:
    x = y = np.linspace(-0.5, 0.5, n)
    ip = self.ip()
    z = ip(x[:, None], y[None, :])
    image = hv.Image(z, bounds=lbrt)
    tris = hv.TriMesh((ip.tri.simplices, self.unscale(ip.tri.points)))
    tris = tris.opts(style=dict(edge_line_alpha=triangles_alpha))
    plot = image * (tris if triangles_alpha else hv.TriMesh(([], [])))
else:
    plot = hv.Image([]) * hv.TriMesh(([], []))
@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 9, 2017

@basnijholt Both hv.TriMesh(([], [])) and hv.TriMesh([]) now work.

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 9, 2017

How can I change the colors of the edges?

(I think) I've tried all style parameters with *_color* but I am not able to get it to work.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 13, 2017

@basnijholt Are your values on the vertices or simplexes? Currently it only allows coloring by values on the simplexes.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 13, 2017

@jlstevens Requesting review again.

@basnijholt

This comment has been minimized.

Contributor

basnijholt commented Dec 13, 2017

Are your values on the vertices or simplexes

The values are at the vertices.

"""
Rasterize is a high-level operation which will rasterize any
Element or combination of Elements supplied as an (Nd)Overlay by
aggregating with the supplied aggregation it with the declared

This comment has been minimized.

@philippjfr

philippjfr Dec 13, 2017

Member

Need to fix this docstring.

supplied, which will ensure the selection is only applied if the
specs match the selected object.
"""
self.edgepaths

This comment has been minimized.

@philippjfr

philippjfr Dec 13, 2017

Member

Will add a comment about this. By default edgepaths are not computed but .select expects it to be.

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

Yes, a comment next to this line explaining it would be a good idea.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 13, 2017

The values are at the vertices.

Okay, I think we could allow coloring by the mean value of the vertices. Interpolation would be nice to support as well but I currently don't see any way to do that in bokeh.

self._edgepaths = edgepaths
@classmethod
def from_points(cls, points):

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

Shouldn't this be from_nodes or from_vertices?

from_points doesn't sound like graph terminology...

This comment has been minimized.

@philippjfr

philippjfr Dec 13, 2017

Member

I think the fact that it is a Graph type is an implementation detail but I agree from_vertices is better.

edge_nonselection_alpha=0.2,
edge_nonselection_line_color='black',
node_nonselection_alpha=0.2,
edge_line_width=1)

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

That is a lot of options!

This comment has been minimized.

@philippjfr

philippjfr Dec 13, 2017

Member

Agreed, but they're mostly for interactive features when you're selecting or hovering over the graph.

bezier = False
# Declares which columns in the data refer to node indices
_node_indices = [0, 1]

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

_node_columns maybe?

path_data['ys'] = [path[:, yidx] for path in edges]
else:
self.warning('Graph edge paths do not match the number of abstract edges '
'and will be skipped')

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

Definitely a warning and not an error condition?

This comment has been minimized.

@philippjfr

philippjfr Dec 13, 2017

Member

Would be happy to make it an error, this has been part of the Graph implementation since first merging it and I've not run into it.

return data, mapping, style
@property
def edge_glyph(self):

This comment has been minimized.

@jlstevens

jlstevens Dec 13, 2017

Member

Little bit too long for what it does imho. Why not compress it a bit?

if not any([self.bezier, self.filled):
   return  'multi_line_1'
return 'bezier_1' if self.bezier else 'patches_1'
@jlstevens

This comment has been minimized.

Member

jlstevens commented Dec 13, 2017

Looks good!

My comments above are fairly minor and the only other thing is I would like to see the from_vertices classmethod mentioned in the two Trimesh element notebooks.

Happy to merge once these small issues are addressed.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Dec 14, 2017

Addressed the comments, added vertex averaging and updated the reference notebooks. Ready for a final review.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Dec 14, 2017

New changes look good and tests have passed. Merging!

@jlstevens jlstevens merged commit f19bdd6 into master Dec 14, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 81.144%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the trimesh branch Dec 18, 2017

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