-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Voronoi diagrams #13517
Comments
a first try |
comment:1
Attachment: trac_#13517_voronoi_first_try.patch.gz I added a patch just to see if I understood correctly how patching and the mercurial export etc works.. |
comment:2
The patch is correct ;-) You should format the examples in the docstring as in http://www.sagemath.org/doc/developer/conventions.html#docstring-markup-with-rest-and-sphinx, then they become tests that you can run with If you want to lay the path for more functionality you should create a class that holds the initial data (the points) and caches the polyhedral cells. Then other methods, for example a Also you don't have to hard-code floating-point doubles as the ring, you could allow exact rationals at the same time. This requires you to figure out what a good ring for the input points is, of course. You could just put them into a
|
comment:3
Thanks for the hints, I will get cracking on this. Replying to @vbraun:
|
Attachment: trac_#13517_new_class_initial_version.patch.gz first working version |
comment:5
I added a first version, which provides the basic functionality |
comment:6
Thanks for the work, i think it is a nice addition. A couple of comments though. -I don't understand why your patch adds the file voronoi_diagram.py.out. Is that made on purpose or is it a mistake? -Unless i am missing something, i think that allowing exact fields (like rationals, or maybe even algebraic reals) should work just the same way. There is really no need to hard code RDF, it would be better to use the same field as the given entry. This should be a really trivial change: just define a self._field as self._points.base_ring(), and then replace RDF by self._field in the rest of your code. |
reviewed |
comment:7
Attachment: trac_#13517_new_class_reviewed.patch.gz thanks for looking at the file.
|
comment:9
Would it be a lot of trouble to introduce also the posibility of using algebraic extensions of QQ as base ring? Arithmetic in those cases may be slow, but i can imagine some situations where it would be important to know the vertices of the diagram in an exact form. |
comment:10
Replying to @miguelmarco:
I don't see how one could easily adapt the algorithm for algebraic extensions of QQ for the following reason: If you really want support for extensions of QQ, either the algorithm of finding the Voronoi-diagram has to be changed, or one has adapt the Polyhedron method. Both are nontrivial tasks, which might make a good upgrade later on. |
comment:11
See also
|
comment:12
Hellooooooooooooo !! Just thought I would add a link toward d3.js here. It's a javascript library that can be used to plot a lot of things, and there's an example of a Voronoi Diagram on its website : http://bl.ocks.org/mbostock/4060366 I wrote a patch (#14953) that uses this library to draw graphs Nathann |
comment:67
There is going to be a conflict with #22496. I guess rebasing will just do the thing. Let's wait some time that the "rc" versions pass... |
comment:68
Dear Moritz, I guess you can now rebase and once the bot gives the green light, I'll give positive review again. |
comment:71
Ok, just rebasing didn't qute work, I did another merge (and squashed). In any case I hope the bot doesn't complain now. I added it to the misc section in the discrete geometry reference. Let's see what the bot thinks. |
comment:73
Hi Moritz, There is a doctest failure:
I guess the output should fix the order. Or change the doctest. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:75
It is better if you use:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:77
Thanks for the tip, Travis! I changed it accordingly. I hope all is well now. |
comment:79
Dear JP, I will set it on positive review on your behalf, as soon as the bot does not complain about the white space changes |
Changed branch from public/13517 to |
a feature request: Voronoi diagrams, see https://groups.google.com/d/topic/sage-devel/JgqyInSu8S8/discussion
A list of things that could be included in an implementation of Voronoi diagrams could be:
Perhaps in the file sage/geometry/triangulation/point_configuration.py
CC: @vbraun @jplab @sagetrac-tmonteil @pelegm
Component: geometry
Keywords: voronoi, delaunay, days84
Author: Moritz Firsching
Branch/Commit:
5dc6bd5
Reviewer: Jean-Philippe Labbé
Issue created by migration from https://trac.sagemath.org/ticket/13517
The text was updated successfully, but these errors were encountered: