Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#12083 closed enhancement (fixed)

Add a TikZ-tex output method for 2d and 3d polytopes

Reported by: jipilab Owned by: mhampton
Priority: minor Milestone: sage-5.13
Component: geometry Keywords: TikZ, polytope
Cc: Merged in: sage-5.13.beta3
Authors: Jean-Philippe Labbé Reviewers: Sébastien Labbé, Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by chapoton)

I recently had to draw polytopes for my work.

Many drawing tools already exist. But none of them satisfied me.

So far, I wrote a method that takes a polytope in 3d and a view-angle (taken from Jmol) and outputs a .tex file containing a TikZ image of the polytope.

The method should also be able to deal with 2-polytopes.

The result is highly customizable: colors, edge style, vertices style, etc...


First review: I made the suggested corrections. They are present in the patch trac12083_tikz_polytope_review.patch

Test passed on version 4.7.2

Apply trac12083_rebased_5.12.patch

Attachments (6)

trac12083_tikz_polytope.patch (394 bytes) - added by jipilab 8 years ago.
Addition of tikz method for polyhedron
trac12083_tikz_polytope.2.patch (19.4 KB) - added by jipilab 8 years ago.
Addition of tikz method for polyhedron
trac12083_tikz_polytope_review.patch (22.4 KB) - added by jipilab 7 years ago.
Apply over the precedent patch
trac_12083_unholy_rebase_to_sage-5.0.patch (24.0 KB) - added by mhampton 7 years ago.
For illustrative purposes only
trac12083_rebased_5.4.1.patch (24.7 KB) - added by jipilab 7 years ago.
Rebased version of the tikz method
trac12083_rebased_5.12.patch (28.0 KB) - added by chapoton 6 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 8 years ago by novoselt

Do you have a ready patch already? Such functionality can be quite awesome in conjunction with SageTeX!

comment:2 Changed 8 years ago by jipilab

I just need to adapt the code a little bit. It is already mostly written and works with 'quite' big examples: A3,B3 and H3 permutahedra.

I just need to sit and do it: it should be there in a week or so...

Changed 8 years ago by jipilab

Addition of tikz method for polyhedron

comment:3 Changed 8 years ago by jipilab

  • Status changed from new to needs_review

Here is the patch. Please test it with your favorite 2d and 3d polytopes!

Changed 8 years ago by jipilab

Addition of tikz method for polyhedron

comment:4 Changed 8 years ago by slabbe

  • Status changed from needs_review to needs_work
  1. Testing the file polyhedra.py creates junk in the working directory.
cd sage-4.8/devel/sage-slabbe/sage/geometry 
sage -t polyhedra.py
ls *.tex 
polytope-tikz.tex       polytope-tikz2.tex      polytope-tikz3.tex               

Saving files in your test is not necessary. I suggest you do the test like this instead :

sage: P1 = polytopes.small_rhombicuboctahedron()   
sage: t = P1.tikz([1,3,5], 175, scale=4)           
sage: type(t)                                      
<type 'str'>                                       
sage: print '\n'.join(t.splitlines()[:4])          
\begin{tikzpicture}%                               
        [x={(-0.939161cm, 0.244762cm)},            
        y={(0.097442cm, -0.482887cm)},             
        z={(0.329367cm, 0.840780cm)},                                          

If you want to keep those line in the doc (a good idea a believe), I suggest you mark them as untested :

sage: open('polytope-tikz2.tex', 'w').write(Image2)    # not tested
  1. You import n but you don't use it. (If you really needed it, I would have suggest to import the equivalent alias numerical_approx instead because the variable n is often used as an integer in the code...) But since you don't use it, just remove the import.
 from subprocess import Popen, PIPE                 
 from sage.misc.all import tmp_filename             
-from sage.misc.functional import norm              
+from sage.misc.functional import norm, n           
 from sage.misc.package import is_package_installed 
  1. Testing the file polyhedra.py is not significantly longer than before : Great!

sage-4.8:

$ sage -t polyhedra.py
sage -t  "devel/sage-slabbe/sage/geometry/polyhedra.py"
         [58.0 s]
----------------------------------------------------------------------                 
All tests passed!
Total time for all tests: 58.0 seconds

sage-4.8 + your patch:

$ sage -t polyhedra.py
sage -t  "devel/sage-slabbe/sage/geometry/polyhedra.py"
         [58.6 s]
----------------------------------------------------------------------                 
All tests passed!
Total time for all tests: 58.6 seconds
  1. You import VectorSpace? but vector could do the job...
 from sage.modules.free_module_element import vector         
+from sage.modules.free_module import VectorSpace            
 from sage.matrix.constructor import matrix, identity_matrix 

In your code, you write :

V = VectorSpace(RDF,3) 
view_vector = V(view)  

I suggest you change this to :

view_vector = vector(RDF, view)
  1. Every methods must have doctests:
$ sage -coverage polyhedra.py
----------------------------------------------------------------------
polyhedra.py
SCORE polyhedra.py: 98% (214 of 217)

Missing doctests:
         * _tikz_2d(self, scale, edge_color, facet_color, opacity, vertex_color, axis):
         * _tikz_2d_in_3d(self, view, rot_angle, scale, edge_color, facet_color, opacity, vertex_color, axis):
         * _tikz_3d_in_3d(self, view, rot_angle, scale, edge_color, facet_color, opacity, vertex_color, axis):

----------------------------------------------------------------------

  1. I tested my script tikz2pdf on the above 3 junk tex files created by the doctest. Pdf files were created without problem. Pictures look great!
  1. Building the documentation creates 2 warnings :
$ sage -docbuild reference html
...
/Users/slabbe/Applications/sage-4.8/local/lib/python2.6/site-packages/sage/geometry/polyhedra.py:docstring
of sage.geometry.polyhedra.Polyhedron.tikz:26: WARNING: Inline literal
start-string without end-string.
/Users/slabbe/Applications/sage-4.8/local/lib/python2.6/site-packages/sage/geometry/polyhedra.py:docstring
of sage.geometry.polyhedra.Polyhedron.tikz:27: WARNING: Inline literal
start-string without end-string.
...

I believe the problem comes from this :

+            2) Select ``Console`;        
+            3) Select the tab ``State`;  
  1. Change this
            It read something like:
                                             
            moveto 0.0 {x y z angle} Scale   

to

            It read something like::
                                             
                moveto 0.0 {x y z angle} Scale   

It will look nicer in the documentation.

  1. I suggest to change the variable name rot_angle to angle.
  1. Input block do not follow the coding convention.

See www.sagemath.org/doc/developer/conventions.html

I suggest you follow this example :

    INPUT:

     - ``x`` - integer (default: 1) the description of the
       argument x goes here.  If it contains multiple lines, all
       the lines after the first need to be indented.

     - ``y`` - integer (default: 2) the ...

    OUTPUT:

    integer -- the ...

So, first say what is the type. Then, say what is the default (inside `` quotes. Then, give the description.

For the output, just say the type and what it is. The part ``tikz_pic`` is not necessary.

  1. The description of view argument could be improved.
- ``view`` -- a list of length 3 representing a vector; 

I know it is a vector. Is it the camera position? Is it the view angle? Is it the axis for the rotation?

  1. I think the type of what you return should be a LatexExpr instead of str.
from sage.misc.latex import LatexExpr

Compare latex(Image1) to latex(LatexExpr(Image1)). Because, sagetex will call latex function on your output...

Bonne fête !

Changed 7 years ago by jipilab

Apply over the precedent patch

comment:5 Changed 7 years ago by jipilab

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Changed 7 years ago by mhampton

For illustrative purposes only

comment:6 Changed 7 years ago by mhampton

I really like this functionality and I thought I'd help by rebasing the patch to sage-5.0, but I have now run out of time. Working on this reminded me of some things I don't like about the Projection class, but I am not sure about the best way to fix them. I think projections should be more accessible from Polyhedron objects, and vice-versa. Anyway, I did complete a patch for sage-5.0 that manages to pass the original tests, but its not very pretty. Its inefficient, and relies on a parent_polyhedron method for projections that might be objectionable. But since it works, it might be helpful in creating a better version.

If no one else does, I might have time to revisit this in a week or so.

comment:7 Changed 7 years ago by novoselt

  • Authors set to Jean-Philippe Labbé
  • Reviewers set to Sébastien Labbé
  • Status changed from needs_review to needs_work

So - which patches have to be applied? I suspect they'll need rebasing after all Volker's refactoring.

comment:8 Changed 7 years ago by chapoton

  • Status changed from needs_work to needs_review

let us try and see

apply trac12083_tikz_polytope.2.patch trac12083_tikz_polytope_review.patch

comment:9 Changed 7 years ago by jipilab

So, today I used the reviewed patch to draw a polytope using the library decorations of tikz.

I realized that I was sloppy to implement the edges; they are drawn twice. This also makes the compilation in LaTeX slower.

I will write a patch for version 5.3 that does not have repetitions (and containing the improvements in the rebased version).

Changed 7 years ago by jipilab

Rebased version of the tikz method

comment:10 Changed 7 years ago by jipilab

So, here is the rebased version of the patch.

I solved a few problems: The vertices and edges appeared twice before (sloppy implementation); There was one vertex not drawn in the first (unholy) rebased version; The facets were badly drawn (problem with vertices naming in the code); The back edges were badly recognized.

I looked at the compiled tikz output examples and now everything seems in order.

All tests passed on 5.4.1

One thing I don't really like though is the fact that we need to pass to projections to access the tikz method. Perhaps a wrapper would do the thing?

comment:11 Changed 7 years ago by jipilab

For the bot:

apply trac12083_rebased_5.4.1.patch

Last edited 7 years ago by jipilab (previous) (diff)

comment:12 Changed 7 years ago by chapoton

There is a problem with the lines

            v1 = self.coords[index1]
            v2 = self.coords[index2]

which appear three times, but the variables v1 and v2 are never used.

comment:13 Changed 7 years ago by chapoton

Moreover, three tests are failing on sage 5.8.beta1

comment:14 Changed 7 years ago by ncohen

  • Status changed from needs_review to needs_work

This patch should probably be rebased. It would be nice to add in the ticket's description which patch are to be applied, too.

Nathann

comment:15 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:16 Changed 6 years ago by chapoton

  • Description modified (diff)
  • Status changed from needs_work to needs_review

here is a rebased patch, passing tests and working on 5.12.beta0

apply only trac12083_rebased_5.12.patch​

comment:17 Changed 6 years ago by chapoton

apply only trac12083_rebased_5.12.patch

comment:18 Changed 6 years ago by vbraun

  • Reviewers changed from Sébastien Labbé to Sébastien Labbé, Volker Braun
  • Status changed from needs_review to positive_review

lgtm

comment:19 Changed 6 years ago by jdemeyer

The PDF doc doesn't build:

! Package inputenc Error: Keyboard character used is undefined
(inputenc)                in inputencoding `utf8'.

See the inputenc package documentation for explanation.
Type  H <return>  for immediate help.
 ...

l.17893 \item[{^^Hegin\{tikzpicture\}\%}] 
                                          \leavevmode

To fix this, use r""" docstrings instead of """.

comment:20 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_work

Changed 6 years ago by chapoton

comment:21 Changed 6 years ago by chapoton

I have made a corrected patch, hopefully.

apply trac12083_rebased_5.12.patch

comment:22 Changed 6 years ago by vbraun

  • Status changed from needs_work to positive_review

Works for me...

comment:23 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.13.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 Changed 6 years ago by kcrisman

I just want to say that I could have used this a couple years ago and instead had to do lots of guess-and-check with graph plots, then convert to xfig... thank you, this will be very useful!

Note: See TracTickets for help on using tickets.