Opened 3 years ago
Closed 3 years ago
#26077 closed defect (fixed)
document and doctest constructing polyhedra over number fields
Reported by:  dimpase  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  geometry  Keywords:  
Cc:  jipilab, mkoeppe, moritz  Merged in:  
Authors:  Dima Pasechnik  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  a962a00 (Commits, GitHub, GitLab)  Commit:  a962a00dd3236e092f1c02c9fa95d7837ee419d7 
Dependencies:  Stopgaps: 
Description (last modified by )
Currently, the only example given in Polyhedron docstring is
When the input contains elements of a Number Field, they require an embedding:: sage: K = NumberField(x^22,'s') sage: s = K.0 sage: L = NumberField(x^32,'t') sage: t = L.0 sage: P = Polyhedron(vertices = [[0,s],[t,0]]) Traceback (most recent call last): ... ValueError: invalid base ring
This is clearly insufficient from online documentation point of view. One has to look either at the html/pdf docs ot at the header of the file sage/geometry/polyhedron/constructor.py
for more examples with embeddings specified.
Change History (10)
comment:1 Changed 3 years ago by
 Cc jipilab mkoeppe moritz added
comment:2 Changed 3 years ago by
here is a small working example
R0.<r0>=QQ[] R1.<r1>=NumberField(r0^22, embedding=1.414213562373095) s = Polyhedron(vertices = [[0,1, 1], [1,r1, 1], [2,1, 1], [7,1, 1]], base_ring=R1)
I'd also have something more interesting, e.g. a regular icosahedron.
comment:3 Changed 3 years ago by
 Description modified (diff)
comment:4 Changed 3 years ago by
Have a look at #17197 (which this present ticket might qualify as a duplicate).
Yes, this is true that polyhedron over number field is now welldocumented. You can have a look at
#22420: Metaticket: Polyhedron: new features and known bugs
and
https://wiki.sagemath.org/OptiPolyGeom
for a roadmap about things on polyhedra. This issue is on the list for a long time. My priority was to offer a wide breath tutorial on polyhedra and use it to then complete the docstring. Perhaps not the right way around, but the tutorial is now available:
http://doc.sagemath.org/html/en/thematic_tutorials/geometry/polyhedra_tutorial.html
thanks to
#22572: Add a thematic tutorial on the polyhedron class
from which you can pick many examples _that work_ under Sage 8.3.
I am sorry if the documentation string still has not been adapted yet.
comment:5 Changed 3 years ago by
 Branch set to u/dimpase/geometry/poly_NF_doc
 Commit set to 1a0c1884e06bfdad1a13adb2965f5353bc072c7b
 Status changed from new to needs_review
Here is a short fix for this. I deliberately constructed an example from scratch,
rather than picking one from polytopes.*
.
I wonder how one can make pointers to the tutorial in the reference manual without mentioning the full URL.
New commits:
1a0c188  add doc for #26077

comment:6 Changed 3 years ago by
 Commit changed from 1a0c1884e06bfdad1a13adb2965f5353bc072c7b to d9d1220104663eddd49a3817ba53e81776cff31a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
d9d1220  add doc for #26077

comment:7 Changed 3 years ago by
some typos fixed; now the links look proper.
comment:8 Changed 3 years ago by
 Branch changed from u/dimpase/geometry/poly_NF_doc to public/26077
 Commit changed from d9d1220104663eddd49a3817ba53e81776cff31a to a962a00dd3236e092f1c02c9fa95d7837ee419d7
 Reviewers set to JeanPhilippe Labbé
Hi,
This looks good to me. I added the spacing convention to the example.
You can put it as positive review on my behalf if you accept the changes.
New commits:
a962a00  Added spacing convention

comment:10 Changed 3 years ago by
 Branch changed from public/26077 to a962a00dd3236e092f1c02c9fa95d7837ee419d7
 Resolution set to fixed
 Status changed from positive_review to closed
Could please someone provide a working example? I'll do the rest...