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: sage-9.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.optimization-online.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 mjo

Work in progress on u/mjo/ticket/29169. Doesn't build or anything yet.

comment:2 Changed 5 months ago by jipilab

  • Cc jipilab added

comment:3 Changed 5 months ago by mjo

  • Dependencies set to 26623

comment:4 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.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 jipilab

  • Branch set to u/mjo/ticket/29169
  • Commit set to 99d77f01268d0d9e381956ba8f0a228a6ab4c19f

New commits:

c9437f1Trac #26623: add global entries for pending cone catalog citations.
792ac2bTrac #26623: add cones.<whatever> shortcuts for common cones.
ddf922cTrac #26623: add sage.geometry.cone_catalog to the documentation index.
f4a4d34Trac #26623: update sage.geometry.cone doctests to use the cone catalog.
9c3bf30Trac #29169: add the Or2020 reference.
6b73284Trac #29169: add the geometry/cone_critical_angles module.
08dd8aaTrac #29169: add the critical_angles and max_angle methods.
99d77f0UNDO ME: add the critical angles module to the doc index.

comment:6 Changed 3 weeks ago by git

  • Commit changed from 99d77f01268d0d9e381956ba8f0a228a6ab4c19f to fc255ba01aa5ce3d2f176aa3f217600c76b7a199

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

1edea71Trac #29169: add the Or2020 reference.
a2a025fTrac #29169: add the geometry/cone_critical_angles module.
4d15305Trac #29169: add the critical_angles and max_angle methods.
fc255baUNDO ME: add the critical angles module to the doc index.

comment:7 Changed 3 weeks ago by git

  • Commit changed from fc255ba01aa5ce3d2f176aa3f217600c76b7a199 to 016564a88267480c9d5f0a684845053a6cfc2576

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

468103bTrac #29169: add the Or2020 reference.
0481039Trac #29169: add the geometry/cone_critical_angles module.
b5ac943Trac #29169: add the critical_angles and max_angle methods.
a3c4535Trac #29169: fix a broken random_cone() test.
016564aUNDO ME: add the critical angles module to the doc index.

comment:8 Changed 3 weeks ago by mjo

  • 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 third-party feedback.

comment:9 Changed 3 weeks ago by mjo

(You can check the docs with sage -docbuild --include-tests-blocks --underscore)

comment:10 Changed 3 weeks ago by chapoton

  • Dependencies changed from 26623 to #26623
  • dependency field was missing #
  • Author field is empty

comment:11 Changed 3 weeks ago by mjo

  • Authors set to Michael Orlitzky
  • Dependencies #26623 deleted

comment:12 Changed 3 weeks ago by chapoton

patchbot complains about coverage and unused import

comment:13 Changed 3 weeks ago by git

  • Commit changed from 016564a88267480c9d5f0a684845053a6cfc2576 to 9e917d3730c1783f9f4507039420383284e394e6

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

468103bTrac #29169: add the Or2020 reference.
7d57406Trac #29169: add the geometry/cone_critical_angles module.
9c4af89Trac #29169: add the critical_angles and max_angle methods.
e4683f4Trac #29169: fix a broken random_cone() test.
9e917d3UNDO ME: add the critical angles module to the doc index.

comment:14 Changed 3 weeks ago by mjo

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 chapoton

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 git

  • Commit changed from 9e917d3730c1783f9f4507039420383284e394e6 to 017cf1ad3a571dfe69c566b3b73057f2931644f6

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

4037b4eTrac #29169: add the geometry/cone_critical_angles module.
dfd587eTrac #29169: add the critical_angles and max_angle methods.
90a1914Trac #29169: fix a broken random_cone() test.
017cf1aUNDO ME: add the critical angles module to the doc index.

comment:17 Changed 2 weeks ago by mjo

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.

Note: See TracTickets for help on using tickets.