Opened 21 months ago
Closed 15 months ago
#28247 closed enhancement (fixed)
Parametrize the cube/hypercube functions in the library of polytopes
Reported by: | selia | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.1 |
Component: | geometry | Keywords: | polytopes, cube, days100 |
Cc: | jipilab, nailuj, gh-kliem | Merged in: | |
Authors: | Sophia Elia | Reviewers: | Jean-Philippe 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 21 months ago by
- Cc gh-kliem added
comment:2 follow-up: ↓ 3 Changed 21 months ago by
comment:3 in reply to: ↑ 2 Changed 21 months 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 16 months ago by
- Milestone changed from sage-8.9 to sage-9.1
Ticket retargeted after milestone closed
comment:5 Changed 15 months ago by
- Branch set to public/28247
- Commit set to 301b3a1bed92baed96171f12e71e58c1de01f8dd
New commits:
301b3a1 | first version
|
comment:6 Changed 15 months 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 15 months ago by
- Commit changed from c6ac53d6e5d89e14091f6f1a5fc10c2020a030cc to f6bcc4ebe49edfc9d78968d479f20e1ea9441069
Branch pushed to git repo; I updated commit sha1. New commits:
f6bcc4e | pyflakes
|
comment:8 Changed 15 months ago by
- Commit changed from f6bcc4ebe49edfc9d78968d479f20e1ea9441069 to 17d8398d783b46e72c1017cb90c796556b3e8caa
Branch pushed to git repo; I updated commit sha1. New commits:
17d8398 | edits
|
comment:9 Changed 15 months ago by
- Status changed from new to needs_review
comment:10 Changed 15 months ago by
comment:11 Changed 15 months ago by
Please make the zero-one string consistent.
comment:12 Changed 15 months 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
ZZor
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 15 months ago by
- Status changed from needs_review to needs_work
comment:14 Changed 15 months 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 15 months 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 15 months ago by
- Status changed from needs_work to needs_review
comment:17 Changed 15 months 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 15 months 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 15 months ago by
@gh-kliem -
Please update the description to include arbitrary intervals.
Can you elaborate on what you mean by including arbitrary intervals?
comment:20 Changed 15 months ago by
The code is more general than the description of the ticket. E.g. you also add the 0/2-cube.
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 15 months ago by
See the very first comment of the ticket. That should somehow go up in the description.
comment:22 Changed 15 months ago by
- Summary changed from Add the 0/1-hypercube to the library of polytopes to Parametrize the cube/hypercube functions in the library of polytopes
comment:23 Changed 15 months ago by
I think you accidentially commited this:
.. csv-table:: - :class: contentstable + ) :class: contentstable :widths: 30 :delim: |
comment:24 Changed 15 months 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 15 months 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 15 months 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 15 months ago by
- Status changed from needs_work to needs_review
comment:28 Changed 15 months ago by
- Reviewers set to Jean-Philippe 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 15 months 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