Implement solid angle measure for two-dimensional simplicial cones

[c13a4060-9513-4188-9d47-dacb5011e673]

Implement the solid angle measure (the plane angle) of a two-dimensional cone, using the dot-product (trigonometry) formula.

For example, the solid angle of the first orthant solid_angle_simplicial_2d([[0,1],[1,0]]) is 1/4.

Later we will add methods for three-dimensional and higher-dimensional cones.

CC: @yuan-zhou @mkoeppe

Component: geometry

Keywords: solid angle

Author: Allison Fitisone, Yuan Zhou

Branch/Commit: u/yzh/implement_solid_angle_measure_for_two_dimensional_simplicial_cones @ d8a5f5a

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/33656

[c13a4060-9513-4188-9d47-dacb5011e673]

Branch: u/gh-allisonfitisone/implement_solid_angle_measure_for_two_dimensional_simplicial_cones

[yuan-zhou]

Commit: c10b244

[yuan-zhou]

Description changed:

--- 
+++ 
@@ -1,3 +1,6 @@
-Implement the solid angle measure (the plane angle) of a two-dimensional cone, using the dot-product formula. Later we will add methods for three-dimensional and higher-dimensional cones.
+Implement the solid angle measure (the plane angle) of a two-dimensional cone, using the dot-product (trigonometry) formula. 

-Examples to be added.
+For example, the solid angle of the first orthant `solid_angle_simplicial_2d([[0,1],[1,0]])`
+is 1/4.
+
+Later we will add methods for three-dimensional and higher-dimensional cones.

[yuan-zhou]

comment:4

a typo in the file description: qudrant --> quadrant

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

e52c2d9

fixed typos

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from c10b244 to e52c2d9

[yuan-zhou]

Changed branch from u/gh-allisonfitisone/implement_solid_angle_measure_for_two_dimensional_simplicial_cones to u/yzh/implement_solid_angle_measure_for_two_dimensional_simplicial_cones

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

5cc0586

change doctests output type

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from e52c2d9 to 5cc0586

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 5cc0586 to 8427e48

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

3c37e7b	check is_Matrix, in the place of hasattr nrows
8427e48	no need to check rank

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 8427e48 to c0fee50

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

0122f46	spaces in doctests
c0fee50	check nonzero row, remove check of rank

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from c0fee50 to b3832eb

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

b3832eb

check nonzero row

[tscrim]

comment:12

Two comments:

• This should be lazily imported.
• Error messages should not end with a period (we follow Python's convention for this).

Otherwise LGTM.

[tscrim]

Reviewer: Travis Scrimshaw

[mkoeppe]

comment:13

I don't think solid_angle_simplicial_2d should really be imported in the global namespace.

It would be better to expose it to the user only via a method of ConvexRationalPolyhedralCone.

[mkoeppe]

comment:14

(as suggested in https://groups.google.com/g/sage-devel/c/K6Cd81dpbPA/m/Q44JFzI5AwAJ)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

8dae533	Remove period from the error messages
b1760ff	do not import solid_angle_simplicial_2d in the global namespace

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b3832eb to b1760ff

[yuan-zhou]

comment:16

Thanks for the feedback! Revised accordingly. I don't think that ConvexRationalPolyhedralCone is the main use case for solid angle computations. It would be pretty useless to go through the ToricLattice but not able to compute the angle of a equilateral triangle.

[mkoeppe]

comment:17

I agree, it would not be good to restrict to the case of rational cones. In https://groups.google.com/g/sage-devel/c/K6Cd81dpbPA/m/Q44JFzI5AwAJ, I also suggest to create methods such as tangent_cone_solid_angle and normal_cone_solid_angle in the class PolyhedronFace (defined in src/sage/geometry/polyhedron/face.py).

[c13a4060-9513-4188-9d47-dacb5011e673]

Changed branch from u/yzh/implement_solid_angle_measure_for_two_dimensional_simplicial_cones to u/gh-allisonfitisone/implement_solid_angle_measure_for_two_dimensional_simplicial_cones

[c13a4060-9513-4188-9d47-dacb5011e673]

Changed commit from b1760ff to fe77320

[c13a4060-9513-4188-9d47-dacb5011e673]

comment:19

changed output to match parent of input

---

New commits:

fe77320

change output type

[yuan-zhou]

comment:20

When P.is_exact() or isinstance(P, sage.rings.abc.SymbolicRing), the return type should be SymbolicConstantsSubring element

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

3c9aa58

modified output type and added tests to docstring

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from fe77320 to 3c9aa58

[mkoeppe]

comment:24

+        sage: solid_angle_simplicial_2d(A).parent()
+        Symbolic Ring
+
+        sage: A = matrix([[0, sqrt(18)], [0, -sqrt(pi)]])
+        sage: solid_angle_simplicial_2d(A)
+        0
+        sage: solid_angle_simplicial_2d(A).parent()
+        Symbolic Constants Subring

If the input is in a symbolic ring, like sqrt(18) is, then the output should be in the same symbolic ring, I think, not the Symbolic Constants Subring.

Symbolic Constants Subring is the right choice when the inputs in an exact ring.

[mkoeppe]

comment:25

Also this:

+    P = A[0][0].parent()

can be replaced by P = A.base_ring().

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 3c9aa58 to 9d5ab98

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

9d5ab98

change output type for input in symbolic ring

[c13a4060-9513-4188-9d47-dacb5011e673]

comment:27

Thanks for the feedback. Please see the recent changes.

[trevorkarn]

comment:28

The patchbot is showing the following: src/sage/geometry/solid_angle.py:194:9 'sage.rings.abc' imported but unused. Does that seem right?

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

1381c23

remove unnecessary line

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 9d5ab98 to 1381c23

[mkoeppe]

comment:30

The parent of the result should only depend on the parent of the input, not the value of the input. So because solid_angle_simplicial_2d(matrix([[0, sqrt(18)], [0, -sqrt(pi)]]) is the 0 of Symbolic Constants Subring, also solid_angle_simplicial_2d(matrix([[0, sqrt(18)], [1, -sqrt(pi)]]) should return a value of Symbolic Constants Subring.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

4673707

wip

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 1381c23 to 4673707

[yuan-zhou]

Changed branch from u/gh-allisonfitisone/implement_solid_angle_measure_for_two_dimensional_simplicial_cones to u/yzh/implement_solid_angle_measure_for_two_dimensional_simplicial_cones

[yuan-zhou]

Changed commit from 4673707 to c48941b

[yuan-zhou]

New commits:

c48941b

output in Symbolic Constants Subring when input is exact

[mkoeppe]

comment:34

How about regrouping the doctests for A and B like this:

sage: [solid_angle_simplicial_2d(matrix(QQ, M)).parent() for M in [A, B]]
[Symbolic Constants Subring, Symbolic Constants Subring]

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

6535715

regroup the doctests for A and B with exact and inexact base rings.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from c48941b to 6535715

[mkoeppe]

comment:36

+    on the parent of the input, but not the value of the input.

This needs ::

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 6535715 to d8a5f5a

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

d8a5f5a

fix doctests typo

[yuan-zhou]

comment:38

Thanks! I'm waiting for the patchbot to run the tests for me :P

Replying to @mkoeppe:

+    on the parent of the input, but not the value of the input.

This needs ::