Opened 2 years ago

Closed 23 months ago

#26077 closed defect (fixed)

document and doctest constructing polyhedra over number fields

Reported by: dimpase Owned by:
Priority: major Milestone: sage-8.4
Component: geometry Keywords:
Cc: jipilab, mkoeppe, moritz Merged in:
Authors: Dima Pasechnik Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: a962a00 (Commits) Commit: a962a00dd3236e092f1c02c9fa95d7837ee419d7
Dependencies: Stopgaps:

Description (last modified by dimpase)

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^2-2,'s')
        sage: s = K.0
        sage: L = NumberField(x^3-2,'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 2 years ago by dimpase

  • Cc jipilab mkoeppe moritz added

Could please someone provide a working example? I'll do the rest...

comment:2 Changed 2 years ago by dimpase

here is a small working example

R0.<r0>=QQ[]
R1.<r1>=NumberField(r0^2-2, 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 2 years ago by dimpase

  • Authors set to Dima Pasechnik
  • Description modified (diff)

comment:4 Changed 2 years ago by jipilab

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 well-documented. You can have a look at

#22420: Meta-ticket: 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 2 years ago by dimpase

  • 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:

1a0c188add doc for #26077

comment:6 Changed 2 years ago by git

  • Commit changed from 1a0c1884e06bfdad1a13adb2965f5353bc072c7b to d9d1220104663eddd49a3817ba53e81776cff31a

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

d9d1220add doc for #26077

comment:7 Changed 2 years ago by dimpase

some typos fixed; now the links look proper.

comment:8 Changed 23 months ago by jipilab

  • Branch changed from u/dimpase/geometry/poly_NF_doc to public/26077
  • Commit changed from d9d1220104663eddd49a3817ba53e81776cff31a to a962a00dd3236e092f1c02c9fa95d7837ee419d7
  • Reviewers set to Jean-Philippe 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:

a962a00Added spacing convention

comment:9 Changed 23 months ago by dimpase

  • Status changed from needs_review to positive_review

Thanks.

comment:10 Changed 23 months ago by vbraun

  • Branch changed from public/26077 to a962a00dd3236e092f1c02c9fa95d7837ee419d7
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.