Add general trig functions via taylor series #371

hugohadfield · 2020-10-21T18:01:38Z

No description provided.

eric-wieser · 2020-10-21T19:40:42Z

clifford/__init__.py

@@ -258,17 +260,73 @@ def general_exp(x, max_order=15):
    for i in range(1, max_order):
        if np.any(np.abs(tmp.value) > _settings._eps):
            tmp = tmp*scaled * (1.0 / i)
-            result += tmp
+            result = result + tmp


What's the motivation for this change?

Motivation was that without this numba will not JIT our general_exp function:

No implementation of function Function(<built-in function iadd>) found for signature: iadd(MultiVector(LayoutType(Layout([1, 1, 1], ids=BasisVectorIds.ordered_integers(3), order=BasisBladeOrder.shortlex(3), names=['', 'e1', 'e2', 'e3', 'e12', 'e13', 'e23', 'e123'])), float64), MultiVector(LayoutType(Layout([1, 1, 1], ids=BasisVectorIds.ordered_integers(3), order=BasisBladeOrder.shortlex(3), names=['', 'e1', 'e2', 'e3', 'e12', 'e13', 'e23', 'e123'])), float64))

clifford/test/test_trig_functions.py

hugohadfield · 2020-10-22T13:11:28Z

clifford/_layout.py

@@ -589,11 +589,11 @@ def _hitzer_inverse(self):
        """
        Performs the inversion operation as described in the paper :cite:`Hitzer_Sangwine_2017`
        """
-        tot = np.sum(self.sig != 0)
+        tot = len(self.sig)


Note this is needed to get the correct behaviour for the tan test, presumably this is the solution for degenerate metric algebras but it would be nice to see a proof of this

clifford/_layout.py

eric-wieser · 2020-10-22T14:46:17Z

clifford/test/test_trig_functions.py

+    @pytest.mark.parametrize('np_func,cmath_func', [(np.sin, cmath.sin),
+                                                    (np.cos, cmath.cos),
+                                                    (np.tan, cmath.tan),
+                                                    (np.sinh, cmath.sinh),
+                                                    (np.cosh, cmath.cosh),
+                                                    (np.tanh, cmath.tanh)])
+    def test_trig_Cl010(self, Cl010element, np_func, cmath_func):
+        """
+        This tests the a clifford algebra isomorphic to the complex numbers
+        """
+        for x in np.linspace(0, 2 * np.pi, 10):
+            for y in np.linspace(0, 2 * np.pi, 10):
+                complex_mv = x * Cl010element[0] + y * Cl010element[1]
+                complex_value = x + 1j * y
+                result = np_func(complex_mv)
+                assert abs(result.value[0] + 1j * result.value[1] - cmath_func(complex_value)) < 1E-10


I sent you down the rabbit hole with cmath_func here - np.cos works on complex numbers just fine - I had falsely assumed your tests were using math.cos.

No worries, I'll remove the cmath import and we can just stick to numpy then :)

clifford/__init__.py

clifford/taylor_expansions.py

clifford/__init__.py

eric-wieser · 2020-10-22T17:25:24Z

clifford/taylor_expansions.py

+
+
+@_numba_utils.njit
+def general_exp(x, max_order=15):


I'd argue you can now remove all the general_ prefixes

clifford/_multivector.py

clifford/numba/_multivector.py

eric-wieser · 2020-12-27T17:31:11Z

This will need a merge / rebase in order to pull in the doc build pinning

hugohadfield · 2020-12-27T18:34:36Z

I rebased and force pushed, hopefully this now works

docs/api/index.rst

eric-wieser · 2020-12-27T20:08:30Z

clifford/_multivector.py

@@ -99,7 +99,25 @@ def _newMV(self, newValue=None, *, dtype: np.dtype = None) -> 'MultiVector':
    # binary

    def exp(self) -> 'MultiVector':


May as well copy this annotation to all the methods below it

clifford/taylor_expansions.py

eric-wieser · 2020-12-27T20:34:05Z

I think my last comment is wondering whether we can make these look more like general_exp, and whether that improves the numerical stability. For instance, we could replace gamma (2*n+1) with iterated division by 2*n*(2*n+1) or whatever the not-off-by-one version is.

We can always use that for a follow-up PR though, now that this PR adds tests

hugohadfield · 2020-12-27T20:38:52Z

I think my last comment is wondering whether we can make these look more like general_exp, and whether that improves the numerical stability. For instance, we could replace gamma (2*n+1) with iterated division by 2*n*(2*n+1) or whatever the not-off-by-one version is.

Yes I think this would be a good idea to look at, I am sure that there will be some useful gains

We can always use that for a follow-up PR though, now that this PR adds tests

This is my thoughts too

eric-wieser

Earlier review was on mobile, apologies. A few small ends to tie up:

I've made some doc suggestions above to try and steer users towards how to use this, and to tweak some formatting.
This needs a release note, and it's easier to add it now than to add it in later
The tests should cover some negative coefficients too!

eric-wieser · 2020-12-27T22:37:55Z

clifford/taylor_expansions.py

+
+This file implements various Taylor expansions for useful functions of multivectors.
+For some algebra signatures there may exist closed forms of these functions which would likely be faster
+and more accurate. Nonetheless having pre-written taylor expansions for the general case is useful.


Suggested change

and more accurate. Nonetheless having pre-written taylor expansions for the general case is useful.

and more accurate. Nonetheless, having pre-written taylor expansions for the general case is useful.

.. note::

Many of these functions are also exposed as :class:`~clifford.MultiVector` methods,

such as :meth:`clifford.MultiVector.sin`. This means that ``mv.sin()`` or even ``np.sin(mv)`` can be used

as a convenient interface to functions in this module, without having to import it directly.

For example::

>>> from clifford.g3 import *

>>> import numpy as np

>>> np.sin(np.pi*e12/4)

# what does this give?

eric-wieser · 2020-12-27T22:40:44Z

clifford/taylor_expansions.py

+@_numba_utils.njit
+def exp(x, max_order=15):
+    """
+    This implements the series expansion of e**mv where mv is a multivector


Suggested change

This implements the series expansion of e**mv where mv is a multivector

This implements the series expansion of :math:`\exp x` where :math:`x` is a multivector

eric-wieser · 2020-12-27T22:40:59Z

clifford/taylor_expansions.py

+def exp(x, max_order=15):
+    """
+    This implements the series expansion of e**mv where mv is a multivector
+    The parameter order is the maximum order of the taylor series to use


Suggested change

The parameter order is the maximum order of the taylor series to use

The parameter `max_order` is the maximum order of the taylor series to use

eric-wieser · 2020-12-27T22:41:21Z

clifford/taylor_expansions.py

+    Note. It would probably be better to implement this as its own taylor series. This function
+    is not JITed as currently we do not overload the truediv operator for multivectors.


Suggested change

Note. It would probably be better to implement this as its own taylor series. This function

is not JITed as currently we do not overload the truediv operator for multivectors.

.. note::

It would probably be better to implement this as its own taylor series. This function

is not JITed as currently we do not overload the truediv operator for multivectors.

eric-wieser · 2020-12-27T22:41:36Z

clifford/taylor_expansions.py

+    Note. It would probably be better to implement this as its own taylor series. This function
+    is not JITed as currently we do not overload the truediv operator for multivectors.


Suggested change

Note. It would probably be better to implement this as its own taylor series. This function

is not JITed as currently we do not overload the truediv operator for multivectors.

.. note::

It would probably be better to implement this as its own taylor series. This function

is not JITed as currently we do not overload the truediv operator for multivectors.

eric-wieser · 2020-12-27T22:42:56Z

clifford/test/test_trig_functions.py

@@ -0,0 +1,86 @@
+
+from .. import Cl


Relative imports in tests are usually a bad idea - tests shouldn't assume they are part of the package.

Suggested change

from .. import Cl

from clifford import Cl

eric-wieser · 2020-12-27T22:44:39Z

clifford/test/test_trig_functions.py

+                                                  (np.cosh, np.sinh),
+                                                  (np.tanh, lambda x: (1 - np.tanh(x)**2))])
+    def test_derivatives(self, element, func, deriv_func):
+        for x in np.linspace(0, 2*np.pi, 10):


How do these fare outside [0, 2*pi]? I'd be inclined to expand this to at least include some negative values,

Suggested change

for x in np.linspace(0, 2*np.pi, 10):

for x in np.linspace(-2*np.pi, 2*np.pi, 13):

eric-wieser · 2020-12-28T13:06:10Z

clifford/taylor_expansions.py

+    >>> np.sin(np.pi*e12/4)
+    >>> # Produces (0.86867^e12)


Suggested change

>>> np.sin(np.pi*e12/4)

>>> # Produces (0.86867^e12)

>>> np.sin(np.pi*e12/4)

(0.86867^e12)

eric-wieser · 2020-12-28T13:08:20Z

docs/changelog.rst

@@ -12,6 +12,10 @@ Changes in 1.4.x
 * Where possible, ``MultiVector``\ s preserve their data type in the dual, and
   the right and left complements.

+ * A new :mod:`clifford.taylor_expansions` is added to implement taylor series of various
+   multivector function, starting with common trigonometric functions. These functions are
+   additionally exposed via a 


Suggested change

additionally exposed via a

additionally exposed via an incomplete sentence that Hugo forgot to write.

eric-wieser · 2020-12-28T13:09:14Z

clifford/taylor_expansions.py

+taylor_expansions (:mod:`clifford.taylor_expansions`)
+=================================================
+
+.. versionadded:: 1.4.1


Suggested change

.. versionadded:: 1.4.1

.. versionadded:: 1.4.0

eric-wieser · 2020-12-28T13:09:22Z

clifford/taylor_expansions.py

+=================================================
+taylor_expansions (:mod:`clifford.taylor_expansions`)
+=================================================


Suggested change

=================================================

taylor_expansions (:mod:`clifford.taylor_expansions`)

=================================================

=====================================================

taylor_expansions (:mod:`clifford.taylor_expansions`)

=====================================================

eric-wieser · 2021-01-11T14:28:37Z

Thanks @hugohadfield 🎉

eric-wieser reviewed Oct 21, 2020

View reviewed changes

clifford/test/test_trig_functions.py Outdated Show resolved Hide resolved