Opened 6 years ago

Closed 5 years ago

#18225 closed enhancement (fixed)

Missing polytopes in the library

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-6.10
Component: geometry Keywords: beginner, polytope
Cc: ncohen, mkoeppe, bedutra, pjxiao Merged in:
Authors: Nico Van Cleemput, Frédéric Chapoton Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 24f0143 (Commits) Commit: 24f0143a0d79134d4b614f68d6ff337c669019f2
Dependencies: #18213 Stopgaps:

Change History (51)

comment:1 Changed 6 years ago by chapoton

  • Branch set to public/ticket/18225
  • Commit set to 160531cb94bfc0f313f1fcebaf3fa0ab6fab70c2

New commits:

160531ctrac #18225 the octahedron

comment:2 Changed 6 years ago by git

  • Commit changed from 160531cb94bfc0f313f1fcebaf3fa0ab6fab70c2 to 373b4ccd4f7e31cf49edaba0c7c728a2ca3bdb5d

Branch pushed to git repo; I updated commit sha1. New commits:

373b4cctrac #18225 tetrahedron

comment:3 Changed 6 years ago by git

  • Commit changed from 373b4ccd4f7e31cf49edaba0c7c728a2ca3bdb5d to 7f5240d1e20430f8a6d4498f9afe2aeb2175610d

Branch pushed to git repo; I updated commit sha1. New commits:

7f5240dtrac #18225 + truncated_tetrahedra

comment:4 Changed 6 years ago by git

  • Commit changed from 7f5240d1e20430f8a6d4498f9afe2aeb2175610d to 8ff9f1eace03f7e5d887fa05a0239287a999b629

Branch pushed to git repo; I updated commit sha1. New commits:

8ff9f1etrac #18225 truncated_cube

comment:5 Changed 6 years ago by chapoton

in fact octahedron was already known as a lattice polytope (lattice_polytope.cross_polytope)

comment:6 Changed 6 years ago by git

  • Commit changed from 8ff9f1eace03f7e5d887fa05a0239287a999b629 to b94a6d0fb7bdb8b0d69494273bb9c352d4d9c56a

Branch pushed to git repo; I updated commit sha1. New commits:

cbef0e8Merge branch 'public/ticket/18225' into 6.8.b2
b94a6d0trac #18225 + truncated octahedron

comment:7 Changed 6 years ago by git

  • Commit changed from b94a6d0fb7bdb8b0d69494273bb9c352d4d9c56a to a4b44f1fa8c0f694fc352b4e3832406d375a76a0

Branch pushed to git repo; I updated commit sha1. New commits:

a4b44f1trac #18225 first try at snub cube (exact is not working, RDF is ok)

comment:8 Changed 6 years ago by git

  • Commit changed from a4b44f1fa8c0f694fc352b4e3832406d375a76a0 to 564ffb02bfbdf9ac7592c2b11a39aeb130a85a5e

Branch pushed to git repo; I updated commit sha1. New commits:

d26990dMerge branch 'public/ticket/18225' into 6.8.b3
564ffb0trac #18225 snub cube exact working using AA but so slow..

comment:9 Changed 6 years ago by git

  • Commit changed from 564ffb02bfbdf9ac7592c2b11a39aeb130a85a5e to 4ebe896dfeb4d1693c55bc174446374cd3468da7

Branch pushed to git repo; I updated commit sha1. New commits:

12bbbf4Merge branch 'public/ticket/18225' into 6.8.b5
4ebe896trac #18225 icosidodecahedron

comment:10 Changed 6 years ago by git

  • Commit changed from 4ebe896dfeb4d1693c55bc174446374cd3468da7 to f1ce189d8268fcc71d700e7c3faa0d1db0a78dfa

Branch pushed to git repo; I updated commit sha1. New commits:

f1ce189trac #18225 truncated dodecahedron

comment:11 Changed 6 years ago by chapoton

  • Keywords polytope added
  • Milestone changed from sage-6.7 to sage-6.8

New commits:

12bbbf4Merge branch 'public/ticket/18225' into 6.8.b5
4ebe896trac #18225 icosidodecahedron
f1ce189trac #18225 truncated dodecahedron

comment:12 Changed 6 years ago by ncohen

Hello Frédéric,

Could we decide something about this Icosidodecahedron thing? My patch at #18775 also contains two graph constructors that I need. Do you know when you will be done with this ticket?

Nathann

comment:13 Changed 6 years ago by chapoton

I will not be able to do anything for the next 2 weeks. Please go ahead if you can find a reviewer for your ticket.

comment:14 Changed 6 years ago by git

  • Commit changed from f1ce189d8268fcc71d700e7c3faa0d1db0a78dfa to 8778b54a75a8171383b56c4e8d3e0ec84ed20007

Branch pushed to git repo; I updated commit sha1. New commits:

8778b54Merge branch 'public/ticket/18225' into 6.9.b1

comment:15 Changed 6 years ago by chapoton

  • Milestone changed from sage-6.8 to sage-6.9

comment:16 Changed 5 years ago by nvcleemp

Hello Frédéric

I needed the rhombicosidodecahedron, so I implemented it. Is it OK if I push it to this ticket, or will this interfere with what you're doing?

Cheers Nico

comment:17 follow-up: Changed 5 years ago by chapoton

Hello,

no problem. You can just add this to the existing branch. I am no longer active on this ticket. Maybe it would be a good time to get it into sage, as a good first step, even if it does not implement everything suggested.

comment:18 in reply to: ↑ 17 ; follow-up: Changed 5 years ago by vdelecroix

Replying to chapoton:

Hello,

no problem. You can just add this to the existing branch. I am no longer active on this ticket. Maybe it would be a good time to get it into sage, as a good first step, even if it does not implement everything suggested.

You are free to split it into several tickets.

Quick remark on the code: some of the constructors do not accept exact/base_ring as arguments. I know that some of them are infinitely slow with exact=True. There are already some of them that have default exact=False that is fine with me if there is a clear warning.

PS: I have code that improves the generic double description algorithm and makes the constructions reasonable over QQbar... but it needs a bit of cleaning.

comment:19 Changed 5 years ago by nvcleemp

Hmm, I seem to be unable to push to this branch, even though it starts with public. This is what I do

$ git push trac public/ticket/18225
git@trac.sagemath.org's password: 
Permission denied, please try again.

The password which I enter is my password for Trac.

comment:20 Changed 5 years ago by chapoton

You should rather set-up a ssh key to push to trac, this is much more convenient.

http://doc.sagemath.org/html/en/developer/trac.html#section-trac-ssh-key

comment:21 Changed 5 years ago by git

  • Commit changed from 8778b54a75a8171383b56c4e8d3e0ec84ed20007 to 81a43954335003f58e3b8da9631e9f66895c5916

Branch pushed to git repo; I updated commit sha1. New commits:

81a4395trac #18225 Added the rhombicosidodecahedron

comment:22 Changed 5 years ago by nvcleemp

  • Description modified (diff)

I changed the description of the ticket a bit in order to give an easy overview of which polytopes are done.

comment:23 Changed 5 years ago by git

  • Commit changed from 81a43954335003f58e3b8da9631e9f66895c5916 to 3a047a708d63521e02519045ac942ff8d41b8a80

Branch pushed to git repo; I updated commit sha1. New commits:

3a047a7trac #18225 truncated icosidodecahedron

comment:24 Changed 5 years ago by nvcleemp

  • Description modified (diff)

comment:25 Changed 5 years ago by chapoton

  • Status changed from new to needs_review

ok, let us turn the ticket to needs review, and try to incorporate it in sage.

comment:26 Changed 5 years ago by chapoton

  • Authors set to Nico Van Cleemput, Frédéric Chapoton
  • Milestone changed from sage-6.9 to sage-6.10

comment:27 in reply to: ↑ 18 Changed 5 years ago by mkoeppe

  • Cc pjxiao added

Replying to vdelecroix:

PS: I have code that improves the generic double description algorithm and makes the constructions reasonable over QQbar... but it needs a bit of cleaning.

Is this code already on a ticket? We'd be quite interested in testing it.

comment:28 Changed 5 years ago by git

  • Commit changed from 3a047a708d63521e02519045ac942ff8d41b8a80 to 6bf290409371af50d04ddd8c7f42a95969a2c58e

Branch pushed to git repo; I updated commit sha1. New commits:

6bf2904Trac #18225 Added truncated icosidodecahedron to the table of contents.

comment:29 Changed 5 years ago by nvcleemp

OK, I quickly had a look at also adding the snub dodecahedron, but since I'm not really familiar with number fields (in Sage), this won't be happening soon, so I think it will indeed be best to just get this into Sage. Unless somebody wants to explain to me how to get the correct field for these coordinates. The information is available at snub dodecahedron coordinates.

comment:30 follow-up: Changed 5 years ago by tscrim

Here is how you can create phi and xi:

sage: Q = QuadraticField(5, 'sqrt5')
sage: sqrt5 = Q.gen()
sage: phi = (1 + sqrt5)/2
sage: A = NumberField(x^3 - 2*x - phi, 'xi')
sage: xi = A.gen()

You can also use RDF for an exact=False input.

Version 0, edited 5 years ago by tscrim (next)

comment:31 in reply to: ↑ 30 Changed 5 years ago by nvcleemp

Replying to tscrim:

Here is how you can create phi and xi:

sage: Q = QuadraticField(5, 'sqrt5')
sage: sqrt5 = Q.gen()
sage: phi = (1 + sqrt5)/2
sage: A = NumberField(x^3 - 2*x - phi, 'xi')
sage: xi = A.gen()

You can also use RDF for an exact=False input.

I had already tried something similar, but then and now I get the same error:

sage: A = NumberField(x^3 -2*x - phi, 'xi')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-44b6c4f1fa2d> in <module>()
----> 1 A = NumberField(x**Integer(3) -Integer(2)*x - phi, 'xi')

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.pyc in NumberField(polynomial, name, check, names, embedding, latex_name, assume_disc_small, maximize_at_primes, structure)
    454         return NumberFieldTower(polynomial, names=name, check=check, embeddings=embedding, latex_names=latex_name, assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structures=structure)
    455 
--> 456     return NumberField_version2(polynomial=polynomial, name=name, check=check, embedding=embedding, latex_name=latex_name, assume_disc_small=assume_disc_small, maximize_at_primes=maximize_at_primes, structure=structure)
    457 
    458 class NumberFieldFactory(UniqueFactory):

/home/nvcleemp/Sage/sage/src/sage/structure/factory.pyx in sage.structure.factory.UniqueFactory.__call__ (/home/nvcleemp/Sage/sage/src/build/cythonized/sage/structure/factory.c:1231)()
    362             False
    363         """
--> 364         key, kwds = self.create_key_and_extra_args(*args, **kwds)
    365         version = self.get_version(sage_version)
    366         return self.get_object(version, key, kwds)

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.pyc in create_key_and_extra_args(self, polynomial, name, check, embedding, latex_name, assume_disc_small, maximize_at_primes, structure)
    530                 polynomial = polynomial.polynomial(QQ)
    531             except (AttributeError, TypeError):
--> 532                 raise TypeError("polynomial (=%s) must be a polynomial." % polynomial)
    533 
    534         # convert polynomial to a polynomial over a field

TypeError: polynomial (=x^3 - 2*x - 1/2*sqrt(5) - 1/2) must be a polynomial.

comment:32 follow-up: Changed 5 years ago by tscrim

Ah, sorry, I forgot to copy in

sage: R.<x> = Q[]

comment:33 in reply to: ↑ 32 Changed 5 years ago by nvcleemp

Replying to tscrim:

Ah, sorry, I forgot to copy in

sage: R.<x> = Q[]

Almost there. It works on the command line, but in the code I get this error:

  File "/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/geometry/polyhedron/library.py", line 1320
    R.<x> = Q[]
      ^
SyntaxError: invalid syntax

And maybe I can also already anticipate and ask another question I will probably have some problem with. How can I specify xi in the exact=False case (i.e., when using RDF)?

comment:34 Changed 5 years ago by chapoton

Thanks for working on that.

For the first question, you should use instead

R = PolynomialRing(Q, 'x')
x = R.gen()

comment:35 Changed 5 years ago by chapoton

For the second question, you can choose the good one among the possible real embeddings of A:

sage: A.embeddings(RDF)

name it 'phi' and then re-define xi as 'phi(xi)'

comment:36 Changed 5 years ago by tscrim

For \xi, you first can define phi = (1 + base_ring(5).sqrt()) / 2 where base_ring = RDF (or whatever was passed in; see icosidodecahedron_V2). Then use the formula on the wikipedia page using foo.nth_root(3) for the cube root.

comment:37 Changed 5 years ago by git

  • Commit changed from 6bf290409371af50d04ddd8c7f42a95969a2c58e to 15c520eee20ffec1776f9c8a8d64aa7c73f68bf9

Branch pushed to git repo; I updated commit sha1. New commits:

0b9b981trac #18225 Added snub dodecahedron (First try: exact is not working)
30d2e29trac #18225 Better name for variable in doctest of truncated_icosidodecahedron
b0f56e4trac #18225 Better name for variable in doctest of rhombicosidodecahedron
3c20369trac #18225 Better name for variable in doctest of truncated_dodecahedron
f2f9351trac #18225 Better name for variable in doctest of icosidodecahedron
15c520etrac #18225 Added snub dodecahedron to the table of contents.

comment:38 follow-up: Changed 5 years ago by nvcleemp

Ok, this is a first version, but exact is not working. I guess this might be the same problem as Frédéric had with the snub cube, but I don't know. Constructing the object gives now problem, but calling f_vector() gives the wrong result. Also calling show() gives the following error message:

sage: sd.show()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-4566439ff143> in <module>()
----> 1 sd.show()

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/geometry/polyhedron/base.pyc in show(self, **kwds)
    706             sage: square.show(point='red')
    707         """
--> 708         self.plot(**kwds).show()
    709 
    710     def _repr_(self):

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/geometry/polyhedron/base.pyc in plot(self, point, line, polygon, wireframe, fill, projection_direction, **kwds)
    678                 raise NotImplementedError('plotting of {0}-dimensional polyhedra not implemented'
    679                                           .format(self.ambient_dim()))
--> 680         return plot_method(*opts)
    681 
    682     def show(self, **kwds):

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/geometry/polyhedron/plot.pyc in render_3d(self, point_opts, line_opts, polygon_opts)
   1246         if isinstance(point_opts, dict):
   1247             point_opts.setdefault('width', 3)
-> 1248             plt += self.render_vertices_3d(**point_opts)
   1249         if isinstance(line_opts, dict):
   1250             line_opts.setdefault('width', 3)

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/geometry/polyhedron/plot.pyc in render_vertices_3d(self, **kwds)
   1080             ((-1.0, -1.0, -1.0), (1.0, 1.0, 1.0))
   1081         """
-> 1082         return point3d(self.coordinates_of(self.points), **kwds)
   1083 
   1084 

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/plot/plot3d/shapes2.pyc in point3d(v, size, **kwds)
   1105             pass
   1106 
-> 1107     A = sum([Point(z, size, **kwds) for z in v])
   1108     A._set_extra_kwds(kwds)
   1109     return A

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/plot/plot3d/shapes2.pyc in __init__(self, center, size, **kwds)
    717         """
    718         PrimitiveObject.__init__(self, **kwds)
--> 719         self.loc = (float(center[0]), float(center[1]), float(center[2]))
    720         self.size = size
    721         self._set_extra_kwds(kwds)

/home/nvcleemp/Sage/sage/src/sage/rings/number_field/number_field_element.pyx in sage.rings.number_field.number_field_element.NumberFieldElement.__float__ (/home/nvcleemp/Sage/sage/src/build/cythonized/sage/rings/number_field/number_field_element.cpp:15292)()
   1409         """
   1410         if self.parent().coerce_embedding() is None:
-> 1411             return float(self.base_ring()(self))
   1412         else:
   1413             c = complex(self)

/home/nvcleemp/Sage/sage/src/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (/home/nvcleemp/Sage/sage/src/build/cythonized/sage/structure/parent.c:9547)()
   1095         if mor is not None:
   1096             if no_extra_args:
-> 1097                 return mor._call_(x)
   1098             else:
   1099                 return mor._call_with_args(x, args, kwds)

/home/nvcleemp/Sage/sage/src/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/nvcleemp/Sage/sage/src/build/cythonized/sage/structure/coerce_maps.c:4425)()
    107                 print type(C), C
    108                 print type(C._element_constructor), C._element_constructor
--> 109             raise
    110 
    111     cpdef Element _call_with_args(self, x, args=(), kwds={}):

/home/nvcleemp/Sage/sage/src/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (/home/nvcleemp/Sage/sage/src/build/cythonized/sage/structure/coerce_maps.c:4332)()
    102         cdef Parent C = self._codomain
    103         try:
--> 104             return C._element_constructor(x)
    105         except Exception:
    106             if print_warnings:

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.pyc in _element_constructor_(self, x)
   1401                     return self._element_class(self, x)
   1402                 x = L(x)
-> 1403             return self._coerce_from_other_number_field(x)
   1404         elif sage.interfaces.gap.is_GapElement(x):
   1405             s = repr(x)

/home/nvcleemp/Sage/sage/local/lib/python2.7/site-packages/sage/rings/number_field/number_field.pyc in _coerce_from_other_number_field(self, x)
   6317             Kgen = F(Kgen)
   6318         else:
-> 6319             raise TypeError("No compatible natural embeddings found for %s and %s"%(KF,LF))
   6320 
   6321         # List of candidates for K(x)

TypeError: No compatible natural embeddings found for Real Lazy Field and Number Field in xi with defining polynomial x^3 - 2*x - 1/2*sqrt5 - 1/2 over its base field

comment:39 Changed 5 years ago by tscrim

Then why don't we just use RDF and not have an exact option for now. We can always give exact versions for both this (and the snub cube) later.

comment:40 Changed 5 years ago by git

  • Commit changed from 15c520eee20ffec1776f9c8a8d64aa7c73f68bf9 to 24f0143a0d79134d4b614f68d6ff337c669019f2

Branch pushed to git repo; I updated commit sha1. New commits:

24f0143trac #18225 Removed exact option for snub dodecahedron since it didn't work.

comment:41 Changed 5 years ago by nvcleemp

  • Description modified (diff)

OK, then this should be the last of the polyhedra for this ticket.

comment:42 Changed 5 years ago by ncohen

Would you mind if I added an automatic index of functions to this file? The list is growing.

Nathann

comment:43 follow-up: Changed 5 years ago by nvcleemp

I certainly wouldn't mind. If I knew how to, I would have done it myself probably.

Cheers Nico

comment:44 in reply to: ↑ 43 Changed 5 years ago by ncohen

I certainly wouldn't mind. If I knew how to, I would have done it myself probably.

I wrote #19482 to fix this very problem.

Nathann

comment:45 Changed 5 years ago by ncohen

(doing it at the moment. Turns out that the 'imported' function are not so easy to sort out)

comment:46 follow-ups: Changed 5 years ago by ncohen

Hello again,

There was a small problem in the index code, which I now fixed at #19596.

If used in the library.py file without the fix, two functions are missing: associahedron and flow_polytope.

We have several ways out (tell me what you prefer):

1) We don't add any index in this ticket, waiting for #19596 to be reviewed

2) We add the index in this ticket, with #19596 as a dependency

3) We add the index in this ticket without #19596 as a dependency, and the two missing elements will appear whenever #19596 will be merged (possibly before the next beta anyway)

Have fun,

Nathann

comment:47 in reply to: ↑ 38 Changed 5 years ago by vdelecroix

Replying to nvcleemp:

Ok, this is a first version, but exact is not working. I guess this might be the same problem as Frédéric had with the snub cube, but I don't know. Constructing the object gives now problem, but calling f_vector() gives the wrong result. Also calling show() gives the following error message:

The problem is that comparisons (ie using < or >) does only work for quadratic field. There is a ticket needing review for that #17830. In the mean time everything is working well (but slowly) with AA the algebraic field:

sage: AA(3).sqrt().sqrt() < AA(2).sqrt()
True

You can notice that number fields for polyhedra are only used with quadratic fields...

comment:48 in reply to: ↑ 46 Changed 5 years ago by nvcleemp

Replying to ncohen:

We have several ways out (tell me what you prefer):

1) We don't add any index in this ticket, waiting for #19596 to be reviewed

2) We add the index in this ticket, with #19596 as a dependency

3) We add the index in this ticket without #19596 as a dependency, and the two missing elements will appear whenever #19596 will be merged (possibly before the next beta anyway)

I don't have any real preference. As long as this ticket gets through fast. ;-) I will try to have a look at #19596 today or tomorrow.

Cheers Nico

comment:49 in reply to: ↑ 46 Changed 5 years ago by tscrim

Replying to ncohen:

3) We add the index in this ticket without #19596 as a dependency, and the two missing elements will appear whenever #19596 will be merged (possibly before the next beta anyway)

It's a separate issue in a way and this could be used as a test for #19596.

comment:50 Changed 5 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

comment:51 Changed 5 years ago by vbraun

  • Branch changed from public/ticket/18225 to 24f0143a0d79134d4b614f68d6ff337c669019f2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.