Opened 5 years ago
Closed 4 years ago
#22572 closed enhancement (fixed)
Add a thematic tutorial on the polyhedron class
Reported by:  jipilab  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  geometry  Keywords:  days84, geometry, tutorial, days88, days93, IMAPolyGeom 
Cc:  moritz, mkoeppe, vdelecroix, tmonteil, chapoton, mmasdeu, tscrim, mforets, SimonKing, novoselt, vklein  Merged in:  
Authors:  JeanPhilippe Labbé, Vincent Delecroix  Reviewers:  Moritz Firsching 
Report Upstream:  N/A  Work issues:  
Branch:  b74ae85 (Commits, GitHub, GitLab)  Commit:  b74ae85343beeaefb536efe7d3c0fd0402d62217 
Dependencies:  #22558, #22565  Stopgaps: 
Description (last modified by )
This ticket adds a thorough tutorial surrounding the polyhedron class.
The tutorial was elaborated during the Sage Days 84 where all participants gave several significant inputs about the tutorial and its content.
FOLLOW UPS:
Change History (39)
comment:1 followup: ↓ 6 Changed 5 years ago by
 Cc SimonKing added
comment:2 Changed 5 years ago by
 Cc novoselt added
comment:3 Changed 5 years ago by
 Dependencies set to #22605, #22498, #22565
comment:4 Changed 5 years ago by
 Branch set to u/jipilab/22572
comment:5 Changed 5 years ago by
 Commit set to fdcb144a5d2847bda231ca415ba5969162a026da
 Dependencies changed from #22605, #22498, #22565 to #22605, #22498, #22565, #22522, #22622, #22558, #22546, #22574, #22575
Last 10 new commits:
4b9192d  Added one more file

5d4faf5  duplicate label

7213306  Edits in titles

81d71bc  added some missing methods

a1ad1af  Added tips and some more methods

7fcfb04  Reorder alphabetically in visualization

46f48fa  Added description of is_ methods

79bb4ce  Corrected a link and added Ehrhart poly

e2aa8a8  Repaired references to objects

fdcb144  Added ref to Perles

comment:6 in reply to: ↑ 1 ; followup: ↓ 7 Changed 5 years ago by
Replying to jipilab:
What is the username of Vincent Klein?
It is vklein
.
I should try to push a version complete "draft version" very soon. Not much is left.
Are you looking for a reviewer for this ticket?
Meanwhile, a question: is it possible that a ref block in rst can not include math mode?
Hmm, I didn't understand :/
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Thanks Marcelo!
My question refers to the fact that I make references to the base classes using :ref:
and in this block, the math
does not appear, like QQ and ZZ. I do not know why...
As for reviewers, you are welcome to review the ticket! I am not completely done yet though... That's why I left it as new. I still want to put some more work in there, but I thought that it is good if people can have a look at it already!
comment:8 Changed 5 years ago by
 Cc vklein added
comment:9 Changed 5 years ago by
 Dependencies changed from #22605, #22498, #22565, #22522, #22622, #22558, #22546, #22574, #22575 to #20887, #22605, #22498, #22565, #22522, #22622, #22558, #22546, #22574, #22575
comment:10 Changed 4 years ago by
 Dependencies changed from #20887, #22605, #22498, #22565, #22522, #22622, #22558, #22546, #22574, #22575 to #20887, #22605, #22498, #22565, #22522, #22622, #22558, #22546,
 Description modified (diff)
comment:11 Changed 4 years ago by
 Keywords days88 added
comment:12 Changed 4 years ago by
you have to remove trailing whitespaces
comment:13 Changed 4 years ago by
I would not order a tutorial by method. This is what the reference manual is about. You should find a "fil rouge". I think the best way to proceed is to have concrete big problems and show how to solve it in Sage step by step.
Furthermore, I think that it is very good to have exercices in thematic tutorials!
EDIT: there are actually a lot of different files:
is_this_polyhedron.rst
lectures.rst
new_from_old.rst
related_objects.rst
tips.rst
visualization.rst
I think it is bad to put these rather generic names. Geometry is not just about polyhedra...
comment:14 Changed 4 years ago by
 Change the title
Tutorials for the Geometry module of Sage
intoPolyhedra
in the filegeometry.rst
 At the very top of the main thematic tutorial page there is a nice distinction between quickrefs, primer and tutorial. It would be good to tag the different files you wrote accordingly.
comment:15 Changed 4 years ago by
 Branch changed from u/jipilab/22572 to public/22572
 Commit changed from fdcb144a5d2847bda231ca415ba5969162a026da to d2dd9f72cb398f9e16b5c45ab1c3d9c1e3516966
 Dependencies changed from #20887, #22605, #22498, #22565, #22522, #22622, #22558, #22546, to #22558, #22565
 Milestone changed from sage7.6 to sage8.1
comment:16 Changed 4 years ago by
I rebased everything on 8.1.beta3 (folding all your commits in 1). The three documents is_this_polyhedron.rst
, new_from_old.rst
and related_objects.rst
are now folded in a unique polyhedra_quickref.rst
.
The organization of the quickref should be discussed...
comment:17 Changed 4 years ago by
 Description modified (diff)
I added some todos that I observed while making the tutorial.
comment:18 Changed 4 years ago by
 Branch changed from public/22572 to public/22572v8.2b5
 Commit changed from d2dd9f72cb398f9e16b5c45ab1c3d9c1e3516966 to 5a09b69e7b0b48072c88766365229f1ca1c1831d
comment:19 Changed 4 years ago by
 Commit changed from 5a09b69e7b0b48072c88766365229f1ca1c1831d to db5e4135aaca2b70fa6ad9ae0981f9c89df52065
Branch pushed to git repo; I updated commit sha1. New commits:
db5e413  Updated to 8.2.b5, fixed many errors

comment:20 Changed 4 years ago by
 Description modified (diff)
 Keywords days93 added
 Milestone changed from sage8.1 to sage8.2
 Status changed from new to needs_review
comment:21 Changed 4 years ago by
 Commit changed from db5e4135aaca2b70fa6ad9ae0981f9c89df52065 to a7b78dda33eacf7ed91d08c78cd549baf56e4ae2
Branch pushed to git repo; I updated commit sha1. New commits:
a7b78dd  Added a ref to the handbook

comment:22 Changed 4 years ago by
 Commit changed from a7b78dda33eacf7ed91d08c78cd549baf56e4ae2 to 31edde6af8a893ddd9ccb3bfd873938a0996b412
Branch pushed to git repo; I updated commit sha1. New commits:
31edde6  Completed a comment

comment:23 Changed 4 years ago by
You should not use a generic name for your file like geometry/lectures.rst
. Your tutorial is specific on polytopes. Note that there is moreover a polytutorial.rst
. Perhaps polytope_quick_tutorial.rst
for the latter and polytope_tutorial.rst
(that way all of them starts with polytope
that helps)?
comment:24 Changed 4 years ago by
Some sided comment:
:math:`\mathbb{R}^d`
could be simplified to`\mathbb{R}^d`
(the default sphinx directive for backstick is math)`\mathbb{R}^d`
could be simplifed in`\RR^d`
(sage comes with some predefined macro)
comment:25 followup: ↓ 27 Changed 4 years ago by
in lectures.rst at the beginning of the first lecture right after the second displayed equation, do you need to ask for the \lambda_i
to be \geq 0
?
comment:26 Changed 4 years ago by
 Commit changed from 31edde6af8a893ddd9ccb3bfd873938a0996b412 to bd6c356e98464b7fa661e4df85a2a2b7e1bd219f
comment:27 in reply to: ↑ 25 Changed 4 years ago by
Replying to sdrewitz:
in lectures.rst at the beginning of the first lecture right after the second displayed equation, do you need to ask for the
\lambda_i
to be\geq 0
?
Yes! Thanks for spotting this!
comment:28 Changed 4 years ago by
 Commit changed from bd6c356e98464b7fa661e4df85a2a2b7e1bd219f to d526f70d1425078e9b617704ec888bc89ddcb0d5
comment:29 followup: ↓ 30 Changed 4 years ago by
I just pushed the suggested changes.
One remark: I prefer how the \mathbf{R}
appears compared to \RR
.
comment:30 in reply to: ↑ 29 Changed 4 years ago by
Replying to jipilab:
I just pushed the suggested changes.
Great :)
One remark: I prefer how the
\mathbf{R}
appears compared to\RR
.
Then propose to change it globally in Sage? This is just a macro. It is better to have it consistent accross documentation rather than good looking on your tutorial.
comment:31 Changed 4 years ago by
 Commit changed from d526f70d1425078e9b617704ec888bc89ddcb0d5 to d7896f8d78766bd364abfc06dd57c9ebc84c9975
Branch pushed to git repo; I updated commit sha1. New commits:
d7896f8  Merge branch 'develop' into 22572

comment:32 Changed 4 years ago by
 Commit changed from d7896f8d78766bd364abfc06dd57c9ebc84c9975 to 597b802e6d0aead26d680870d43052cf09e9b2f9
Branch pushed to git repo; I updated commit sha1. New commits:
597b802  Merge branch sage8.2.rc1 into 22572

comment:33 Changed 4 years ago by
 Reviewers set to Moritz Firsching
 Status changed from needs_review to needs_work
Thanks for the tutorial!
I only have a few remarks/corrections
 has the ambient space > as the ambient space
 should the explanation for "is_compact" perhaps mention the word "bounded"?
 add a link for "composite_field" (and change the word to "composite_fields")
 the reason why we cant do computations in the symbolic ring is given as because it is "not exact". Perhaps we could simply state that it is not implemented. (Computations in RDF are also "not exact")
 access to vertices ... > access vertices ...
 remove the part "clearly (!)" (or is this a joke I don't get?)
 in the sage_input example, I suggest to change whats there to
sage: Cube = polytopes.cube() sage: TCube = Cube.truncation().dilation(1/2) sage: sage_input(TCube)
That is add "dilation" to avoid "QQ(1)" many times, which doesn't look pretty.
comment:34 Changed 4 years ago by
 Commit changed from 597b802e6d0aead26d680870d43052cf09e9b2f9 to b74ae85343beeaefb536efe7d3c0fd0402d62217
comment:35 Changed 4 years ago by
 Status changed from needs_work to needs_review
Salut Moritz,
Thanks a lot for the review. I made the corrections you suggested.
Hopefully, the bot likes it.
comment:36 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:37 Changed 4 years ago by
 Keywords IMAPolyGeom added
comment:38 Changed 4 years ago by
 Milestone changed from sage8.2 to sage8.3
comment:39 Changed 4 years ago by
 Branch changed from public/22572v8.2b5 to b74ae85343beeaefb536efe7d3c0fd0402d62217
 Resolution set to fixed
 Status changed from positive_review to closed
What is the username of Vincent Klein?
I should try to push a version complete "draft version" very soon. Not much is left.
Meanwhile, a question: is it possible that a ref block in rst can not include math mode?