Opened 3 years ago
Closed 3 years ago
#28247 closed enhancement (fixed)
Parametrize the cube/hypercube functions in the library of polytopes
Reported by:  selia  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  geometry  Keywords:  polytopes, cube, days100 
Cc:  jipilab, nailuj, ghkliem  Merged in:  
Authors:  Sophia Elia  Reviewers:  JeanPhilippe Labbé, Jonathan Kliem 
Report Upstream:  N/A  Work issues:  
Branch:  6da26d9 (Commits, GitHub, GitLab)  Commit:  6da26d9b9020fd02a2ed43163e2c4f3550bf3e92 
Dependencies:  Stopgaps: 
Description
Currently, the polytopes.cube
and polytopes.hypercube
s use vertex coordinates +/1. It would be nice to have the 0/1 cube and the 0/1 hypercube. This ticket adds them to the library of polytopes.
Change History (29)
comment:1 Changed 3 years ago by
 Cc ghkliem added
comment:2 followup: ↓ 3 Changed 3 years ago by
comment:3 in reply to: ↑ 2 Changed 3 years ago by
Replying to vdelecroix:
Don't you want to parametrize the already existing
cube/hypercube
functions? Givenx
andy
and more generallyx1,y1, ..., xd,yd
you can create the polytope[x1,y1] x [x2,y2] x ... x [xd,yd]
Yes, yes, that's the idea; making it inside cube
and hypercube
was the intended plan.
I am not sure whether to also include:
[x1,y1] x [x2,y2] x ... x [xd,yd]
as well in there. One could try.
There are many ways to interpret x
above. There is polytopes.zonotope
which does:
sage: corner = vector([1,2,3,4]) sage: directions = diagonal_matrix([10,9,8,7]).columns() sage: z = polytopes.zonotope(directions).translation(corner) sage: z.vertices_list() [[1, 2, 3, 4], [1, 2, 3, 11], [1, 2, 11, 4], [1, 2, 11, 11], [1, 11, 3, 4], [1, 11, 3, 11], [1, 11, 11, 4], [1, 11, 11, 11], [11, 2, 3, 4], [11, 2, 3, 11], [11, 2, 11, 4], [11, 2, 11, 11], [11, 11, 3, 4], [11, 11, 3, 11], [11, 11, 11, 4], [11, 11, 11, 11]] sage: hc = polytopes.hypercube(4) sage: z.is_combinatorially_isomorphic(hc) True
That said, I see that it might be nice to have this directly possible inside of cube
and hypercube
...
comment:4 Changed 3 years ago by
 Milestone changed from sage8.9 to sage9.1
Ticket retargeted after milestone closed
comment:5 Changed 3 years ago by
 Branch set to public/28247
 Commit set to 301b3a1bed92baed96171f12e71e58c1de01f8dd
New commits:
301b3a1  first version

comment:6 Changed 3 years ago by
 Commit changed from 301b3a1bed92baed96171f12e71e58c1de01f8dd to c6ac53d6e5d89e14091f6f1a5fc10c2020a030cc
Branch pushed to git repo; I updated commit sha1. New commits:
c6ac53d  fixing doctests

comment:7 Changed 3 years ago by
 Commit changed from c6ac53d6e5d89e14091f6f1a5fc10c2020a030cc to f6bcc4ebe49edfc9d78968d479f20e1ea9441069
Branch pushed to git repo; I updated commit sha1. New commits:
f6bcc4e  pyflakes

comment:8 Changed 3 years ago by
 Commit changed from f6bcc4ebe49edfc9d78968d479f20e1ea9441069 to 17d8398d783b46e72c1017cb90c796556b3e8caa
Branch pushed to git repo; I updated commit sha1. New commits:
17d8398  edits

comment:9 Changed 3 years ago by
 Status changed from new to needs_review
comment:10 Changed 3 years ago by
comment:11 Changed 3 years ago by
Please make the zeroone string consistent.
comment:12 Changed 3 years ago by
Looks mostly good. Some comments:
 In the docstring of
cube
remove empty line and unindent the output ofcc.vertices_list()
. three intervals of length 2
: Do not specify the length of interval. Instead of
vectors in
\RR^{{\text{dim}} one could specify the length or dimension of the vectors, to account for the polyhedron to be constructed in ZZ or QQ`. }  I would maybe move the test of the error
the dimension of the hypercube must match the number of intervals
to tests. I don't think it is very interesting for the user. 
 elif isinstance(intervals,str): + elif isinstance(intervals, str):
 Missing parenthesis
 cp = list(itertools.product(*intervals) + cp = list(itertools.product(*intervals))
comment:13 Changed 3 years ago by
 Status changed from needs_review to needs_work
comment:14 Changed 3 years ago by
 Commit changed from 17d8398d783b46e72c1017cb90c796556b3e8caa to 8967a0f356bcf51adacadebec8274dd34b7a3e97
Branch pushed to git repo; I updated commit sha1. New commits:
8967a0f  adding edits

comment:15 Changed 3 years ago by
 Commit changed from 8967a0f356bcf51adacadebec8274dd34b7a3e97 to a9d7a714184009d71de654b8519919dde11381e4
Branch pushed to git repo; I updated commit sha1. New commits:
a9d7a71  fixing variable name

comment:16 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:17 Changed 3 years ago by
Some minor commens:
 Please update the description to include arbitrary intervals.
 The
INPUT
block should also mention the behavior for the inputNone
.  Instead of
list/tuple/iterable of length
I would just saylist
, especially as an iterable does not even work in general (len
does not work).  A string should be written as code:
  'zero_one'  (string). Return the `0/1`cube. +  ``'zero_one'``  Return the `0/1`cube.
 Usually
INPUT
blocks do not have periods, but I don't know if you should change this for this method now.
comment:18 Changed 3 years ago by
 Commit changed from a9d7a714184009d71de654b8519919dde11381e4 to 696dbe01c1d84f662fc8362b898cb7fc4d250460
Branch pushed to git repo; I updated commit sha1. New commits:
696dbe0  fixing comments

comment:19 Changed 3 years ago by
@ghkliem 
Please update the description to include arbitrary intervals.
Can you elaborate on what you mean by including arbitrary intervals?
comment:20 Changed 3 years ago by
The code is more general than the description of the ticket. E.g. you also add the 0/2cube.
It would be nice if the description reflects that one can now get the [a_1,b_1],…,[a_n,b_n] Cube.
comment:21 Changed 3 years ago by
See the very first comment of the ticket. That should somehow go up in the description.
comment:22 Changed 3 years ago by
 Summary changed from Add the 0/1hypercube to the library of polytopes to Parametrize the cube/hypercube functions in the library of polytopes
comment:23 Changed 3 years ago by
I think you accidentially commited this:
.. csvtable::  :class: contentstable + ) :class: contentstable :widths: 30 :delim: 
comment:24 Changed 3 years ago by
 Commit changed from 696dbe01c1d84f662fc8362b898cb7fc4d250460 to ce4627f094a43a54725c7374f44947e7c91ca067
Branch pushed to git repo; I updated commit sha1. New commits:
ce4627f  removing parenthesis

comment:25 Changed 3 years ago by
 Status changed from needs_review to needs_work
There is a double colon after EXAMPLES
cube, followed by another comment. This makes building the documentation fail. Please remove it:
 EXAMPLES:: + EXAMPLES: Return the `\pm 1`cube::
comment:26 Changed 3 years ago by
 Commit changed from ce4627f094a43a54725c7374f44947e7c91ca067 to 6da26d9b9020fd02a2ed43163e2c4f3550bf3e92
Branch pushed to git repo; I updated commit sha1. New commits:
6da26d9  removing colon

comment:27 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:28 Changed 3 years ago by
 Reviewers set to JeanPhilippe Labbé, Jonathan Kliem
 Status changed from needs_review to positive_review
LGTM.
@JP: I put you as reviewer, I hope that is ok.
comment:29 Changed 3 years ago by
 Branch changed from public/28247 to 6da26d9b9020fd02a2ed43163e2c4f3550bf3e92
 Resolution set to fixed
 Status changed from positive_review to closed
Don't you want to parametrize the already existing
cube/hypercube
functions? Givenx
andy
and more generallyx1,y1, ..., xd,yd
you can create the polytope