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:  sage6.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, GitHub, GitLab)  Commit:  24f0143a0d79134d4b614f68d6ff337c669019f2 
Dependencies:  #18213  Stopgaps: 
Description (last modified by )
Sage forgot the existence of some polyhedras!
Platonic (or regular) solids:
Archimedian (or semiregular) solids:
truncated tetrahedrontruncated cubetruncated octahedron (truncated tetratetrahedron)snub cube (snub cuboctahedron)icosidodecahedrontruncated icosahedronrhombicosidodecahedron (small rhombicosidodecahedron)truncated icosidodecahedron (great rhombicosidodecahedron)snub dodecahedron (snub icosidodecahedron)
And possibly:
Change History (51)
comment:1 Changed 6 years ago by
 Branch set to public/ticket/18225
 Commit set to 160531cb94bfc0f313f1fcebaf3fa0ab6fab70c2
comment:2 Changed 6 years ago by
 Commit changed from 160531cb94bfc0f313f1fcebaf3fa0ab6fab70c2 to 373b4ccd4f7e31cf49edaba0c7c728a2ca3bdb5d
Branch pushed to git repo; I updated commit sha1. New commits:
373b4cc  trac #18225 tetrahedron

comment:3 Changed 6 years ago by
 Commit changed from 373b4ccd4f7e31cf49edaba0c7c728a2ca3bdb5d to 7f5240d1e20430f8a6d4498f9afe2aeb2175610d
Branch pushed to git repo; I updated commit sha1. New commits:
7f5240d  trac #18225 + truncated_tetrahedra

comment:4 Changed 6 years ago by
 Commit changed from 7f5240d1e20430f8a6d4498f9afe2aeb2175610d to 8ff9f1eace03f7e5d887fa05a0239287a999b629
Branch pushed to git repo; I updated commit sha1. New commits:
8ff9f1e  trac #18225 truncated_cube

comment:5 Changed 6 years ago by
in fact octahedron was already known as a lattice polytope (lattice_polytope.cross_polytope)
comment:6 Changed 6 years ago by
 Commit changed from 8ff9f1eace03f7e5d887fa05a0239287a999b629 to b94a6d0fb7bdb8b0d69494273bb9c352d4d9c56a
comment:7 Changed 6 years ago by
 Commit changed from b94a6d0fb7bdb8b0d69494273bb9c352d4d9c56a to a4b44f1fa8c0f694fc352b4e3832406d375a76a0
Branch pushed to git repo; I updated commit sha1. New commits:
a4b44f1  trac #18225 first try at snub cube (exact is not working, RDF is ok)

comment:8 Changed 6 years ago by
 Commit changed from a4b44f1fa8c0f694fc352b4e3832406d375a76a0 to 564ffb02bfbdf9ac7592c2b11a39aeb130a85a5e
comment:9 Changed 6 years ago by
 Commit changed from 564ffb02bfbdf9ac7592c2b11a39aeb130a85a5e to 4ebe896dfeb4d1693c55bc174446374cd3468da7
comment:10 Changed 6 years ago by
 Commit changed from 4ebe896dfeb4d1693c55bc174446374cd3468da7 to f1ce189d8268fcc71d700e7c3faa0d1db0a78dfa
Branch pushed to git repo; I updated commit sha1. New commits:
f1ce189  trac #18225 truncated dodecahedron

comment:11 Changed 6 years ago by
 Keywords polytope added
 Milestone changed from sage6.7 to sage6.8
comment:12 Changed 6 years ago by
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
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
 Commit changed from f1ce189d8268fcc71d700e7c3faa0d1db0a78dfa to 8778b54a75a8171383b56c4e8d3e0ec84ed20007
Branch pushed to git repo; I updated commit sha1. New commits:
8778b54  Merge branch 'public/ticket/18225' into 6.9.b1

comment:15 Changed 6 years ago by
 Milestone changed from sage6.8 to sage6.9
comment:16 Changed 5 years ago by
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 followup: ↓ 18 Changed 5 years ago by
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 ; followup: ↓ 27 Changed 5 years ago by
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
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
You should rather setup a ssh key to push to trac, this is much more convenient.
http://doc.sagemath.org/html/en/developer/trac.html#sectiontracsshkey
comment:21 Changed 5 years ago by
 Commit changed from 8778b54a75a8171383b56c4e8d3e0ec84ed20007 to 81a43954335003f58e3b8da9631e9f66895c5916
Branch pushed to git repo; I updated commit sha1. New commits:
81a4395  trac #18225 Added the rhombicosidodecahedron

comment:22 Changed 5 years ago by
 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
 Commit changed from 81a43954335003f58e3b8da9631e9f66895c5916 to 3a047a708d63521e02519045ac942ff8d41b8a80
Branch pushed to git repo; I updated commit sha1. New commits:
3a047a7  trac #18225 truncated icosidodecahedron

comment:24 Changed 5 years ago by
 Description modified (diff)
comment:25 Changed 5 years ago by
 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
 Milestone changed from sage6.9 to sage6.10
comment:27 in reply to: ↑ 18 Changed 5 years ago by
 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
 Commit changed from 3a047a708d63521e02519045ac942ff8d41b8a80 to 6bf290409371af50d04ddd8c7f42a95969a2c58e
Branch pushed to git repo; I updated commit sha1. New commits:
6bf2904  Trac #18225 Added truncated icosidodecahedron to the table of contents.

comment:29 Changed 5 years ago by
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 followup: ↓ 31 Changed 5 years ago by
Here is how you can create phi
and xi
:
sage: Q = QuadraticField(5, 'sqrt5') sage: sqrt5 = Q.gen() sage: phi = (1 + sqrt5)/2 sage: R.<x> = Q[] sage: A = NumberField(x^3  2*x  phi, 'xi') sage: xi = A.gen()
You can also use RDF
for an exact=False
input.
Edit  Missed one command
comment:31 in reply to: ↑ 30 Changed 5 years ago by
Replying to tscrim:
Here is how you can create
phi
andxi
: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 anexact=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) <ipythoninput544b6c4f1fa2d> in <module>() > 1 A = NumberField(x**Integer(3) Integer(2)*x  phi, 'xi') /home/nvcleemp/Sage/sage/local/lib/python2.7/sitepackages/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/sitepackages/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 followup: ↓ 33 Changed 5 years ago by
Ah, sorry, I forgot to copy in
sage: R.<x> = Q[]
comment:33 in reply to: ↑ 32 Changed 5 years ago by
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/sitepackages/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
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
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 redefine xi as 'phi(xi)'
comment:36 Changed 5 years ago by
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
 Commit changed from 6bf290409371af50d04ddd8c7f42a95969a2c58e to 15c520eee20ffec1776f9c8a8d64aa7c73f68bf9
Branch pushed to git repo; I updated commit sha1. New commits:
0b9b981  trac #18225 Added snub dodecahedron (First try: exact is not working)

30d2e29  trac #18225 Better name for variable in doctest of truncated_icosidodecahedron

b0f56e4  trac #18225 Better name for variable in doctest of rhombicosidodecahedron

3c20369  trac #18225 Better name for variable in doctest of truncated_dodecahedron

f2f9351  trac #18225 Better name for variable in doctest of icosidodecahedron

15c520e  trac #18225 Added snub dodecahedron to the table of contents.

comment:38 followup: ↓ 47 Changed 5 years ago by
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) <ipythoninput44566439ff143> in <module>() > 1 sd.show() /home/nvcleemp/Sage/sage/local/lib/python2.7/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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
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
 Commit changed from 15c520eee20ffec1776f9c8a8d64aa7c73f68bf9 to 24f0143a0d79134d4b614f68d6ff337c669019f2
Branch pushed to git repo; I updated commit sha1. New commits:
24f0143  trac #18225 Removed exact option for snub dodecahedron since it didn't work.

comment:41 Changed 5 years ago by
 Description modified (diff)
OK, then this should be the last of the polyhedra for this ticket.
comment:42 Changed 5 years ago by
Would you mind if I added an automatic index of functions to this file? The list is growing.
Nathann
comment:43 followup: ↓ 44 Changed 5 years ago by
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
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
(doing it at the moment. Turns out that the 'imported' function are not so easy to sort out)
comment:46 followups: ↓ 48 ↓ 49 Changed 5 years ago by
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
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 callingshow()
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
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
comment:50 Changed 5 years ago by
 Reviewers set to Volker Braun
 Status changed from needs_review to positive_review
comment:51 Changed 5 years ago by
 Branch changed from public/ticket/18225 to 24f0143a0d79134d4b614f68d6ff337c669019f2
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
trac #18225 the octahedron