Opened 5 months ago
Last modified 2 weeks ago
#29169 needs_review enhancement
Maximal and critical angles for cones
Reported by:  mjo  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  geometry  Keywords:  
Cc:  jipilab  Merged in:  
Authors:  Michael Orlitzky  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/mjo/ticket/29169 (Commits)  Commit:  017cf1ad3a571dfe69c566b3b73057f2931644f6 
Dependencies:  Stopgaps: 
Description
My paper with two algorithms for finding the maximal/critical angles between polyhedral convex cones was accepted:
http://www.optimizationonline.org/DB_HTML/2019/01/7048.html
I've already written these algorithms in sage, so it would be nice to add them to the library. I just need to clean up some of the documentation.
Change History (17)
comment:1 Changed 5 months ago by
comment:2 Changed 5 months ago by
 Cc jipilab added
comment:3 Changed 5 months ago by
 Dependencies set to 26623
comment:4 Changed 3 months ago by
 Milestone changed from sage9.1 to sage9.2
Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.
comment:5 Changed 3 months ago by
 Branch set to u/mjo/ticket/29169
 Commit set to 99d77f01268d0d9e381956ba8f0a228a6ab4c19f
New commits:
c9437f1  Trac #26623: add global entries for pending cone catalog citations.

792ac2b  Trac #26623: add cones.<whatever> shortcuts for common cones.

ddf922c  Trac #26623: add sage.geometry.cone_catalog to the documentation index.

f4a4d34  Trac #26623: update sage.geometry.cone doctests to use the cone catalog.

9c3bf30  Trac #29169: add the Or2020 reference.

6b73284  Trac #29169: add the geometry/cone_critical_angles module.

08dd8aa  Trac #29169: add the critical_angles and max_angle methods.

99d77f0  UNDO ME: add the critical angles module to the doc index.

comment:6 Changed 3 weeks ago by
 Commit changed from 99d77f01268d0d9e381956ba8f0a228a6ab4c19f to fc255ba01aa5ce3d2f176aa3f217600c76b7a199
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1edea71  Trac #29169: add the Or2020 reference.

a2a025f  Trac #29169: add the geometry/cone_critical_angles module.

4d15305  Trac #29169: add the critical_angles and max_angle methods.

fc255ba  UNDO ME: add the critical angles module to the doc index.

comment:7 Changed 3 weeks ago by
 Commit changed from fc255ba01aa5ce3d2f176aa3f217600c76b7a199 to 016564a88267480c9d5f0a684845053a6cfc2576
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
468103b  Trac #29169: add the Or2020 reference.

0481039  Trac #29169: add the geometry/cone_critical_angles module.

b5ac943  Trac #29169: add the critical_angles and max_angle methods.

a3c4535  Trac #29169: fix a broken random_cone() test.

016564a  UNDO ME: add the critical angles module to the doc index.

comment:8 Changed 3 weeks ago by
 Status changed from new to needs_review
I can keep tweaking the documentation forever, but I think this is in good condition to get some thirdparty feedback.
comment:9 Changed 3 weeks ago by
(You can check the docs with sage docbuild includetestsblocks underscore
)
comment:10 Changed 3 weeks ago by
 Dependencies changed from 26623 to #26623
 dependency field was missing #
 Author field is empty
comment:11 Changed 3 weeks ago by
 Dependencies #26623 deleted
comment:12 Changed 3 weeks ago by
patchbot complains about coverage and unused import
comment:13 Changed 3 weeks ago by
 Commit changed from 016564a88267480c9d5f0a684845053a6cfc2576 to 9e917d3730c1783f9f4507039420383284e394e6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
468103b  Trac #29169: add the Or2020 reference.

7d57406  Trac #29169: add the geometry/cone_critical_angles module.

9c4af89  Trac #29169: add the critical_angles and max_angle methods.

e4683f4  Trac #29169: fix a broken random_cone() test.

9e917d3  UNDO ME: add the critical angles module to the doc index.

comment:14 Changed 3 weeks ago by
The unused import was legit (and is now fixed), but the coverage warning is for the critical_angles
and max_angle
functions that are wrapped by the methods of the same name in the cone class. All of the documentation (and unit tests) is on the cone methods, because that's what people will use.
(I can add some trivial tests if a happy coverage report is worth it, but so far I have avoided it as it's a complete waste of CPU time.)
comment:15 Changed 3 weeks ago by
I think that coverage is mandatory, sorry. Very simple tests would do, and you can tag them with #indirect doctest
if they do not use the method directly.
More annoying, the latest patchbot reports a Timed Out
, which is not acceptable.
comment:16 Changed 2 weeks ago by
 Commit changed from 9e917d3730c1783f9f4507039420383284e394e6 to 017cf1ad3a571dfe69c566b3b73057f2931644f6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4037b4e  Trac #29169: add the geometry/cone_critical_angles module.

dfd587e  Trac #29169: add the critical_angles and max_angle methods.

90a1914  Trac #29169: fix a broken random_cone() test.

017cf1a  UNDO ME: add the critical angles module to the doc index.

comment:17 Changed 2 weeks ago by
I think that will make patchbot happy. I added some trivial tests where they were absent, and fixed the timeout issue (fingers crossed) by adding # long time
in a bunch of places and reducing the max test size in others.
Work in progress on
u/mjo/ticket/29169
. Doesn't build or anything yet.