Fix heterogeneous sdop.Add (#157)

Before this change, any addition other than `Sdop + Sdop` would raise a NameError, since the variable `coef` is not defined in the `elif` and `else` branch.

The intent of the old code was fairly clear - it was to insert a multiplicative term for `sdop2.Ga.Pdop_identity`, such that `(sdop + 2)(x) == sdop(x) + 2*x`, which is reflected by the repr in a shell.
This change just copies the code from `Dop`, which succeeds in doing exactly the same thing

This also adds a bunch more tests for both Sdop and Dop operations

Author: eric-wieser

galgebra/mv.py

@@ -1609,28 +1609,20 @@ def Add(sdop1, sdop2):
         if isinstance(sdop1, Sdop) and isinstance(sdop2, Sdop):
             if sdop1.Ga != sdop2.Ga:
                 raise ValueError('In Sdop.Add sdop1.Ga != sdop2.Ga.')
-
-            sdop_sum = Sdop(_merge_terms(sdop1.terms, sdop2.terms), ga=sdop1.Ga)
-        elif isinstance(sdop1, Sdop):
-            coefs, pdiffs = list(zip(*sdop1.terms))
-            if sdop1.Ga.Pdop_identity in pdiffs:
-                index = pdiffs.index(sdop1.Ga.Pdop_identity)
-                coef[index] += sdop2
-            else:
-                coef.append(sdop2)
-                pdiff.append(sdop1.Ga.Pdop_identity)
-            return Sdop(coefs, pdiffs, ga=sdop1.Ga)
+            return Sdop(_merge_terms(sdop1.terms, sdop2.terms), ga=sdop1.Ga)
         else:
-            coefs, pdiffs = list(zip(*sdop2.terms))
-            if sdop2.Ga.Pdop_identity in pdiffs:
-                index = pdiffs.index(sdop2.Ga.Pdop_identity)
-                coef[index] += sdop1
+            # convert values to multiplicative operators
+            if isinstance(sdop1, Sdop):
+                if not isinstance(sdop2, Mv):
+                    sdop2 = sdop1.Ga.mv(sdop2)
+                sdop2 = Sdop([(sdop2, sdop1.Ga.Pdop_identity)], ga=sdop1.Ga)
+            elif isinstance(sdop2, Sdop):
+                if not isinstance(sdop1, Mv):
+                    sdop1 = sdop2.Ga.mv(sdop1)
+                sdop1 = Sdop([(sdop1, sdop2.Ga.Pdop_identity)], ga=sdop2.Ga)
             else:
-                coef.append(sdop1)
-                pdiff.append(sdop2.Ga.Pdop_identity)
-            sdop_sum = Sdop(coefs, pdiffs, ga=sdop2.Ga)
-
-        return Sdop.consolidate_coefs(sdop_sum)
+                raise TypeError("Neither argument is a Dop instance")
+            return Sdop.Add(sdop1, sdop2)
 
     def __eq__(self, sdop):
         if isinstance(sdop, Sdop):
@@ -2033,14 +2025,17 @@ def Add(dop1, dop2):
 
             return Dop(_merge_terms(dop1.terms, dop2.terms), cmpflg=dop1.cmpflg, ga=dop1.Ga)
         else:
-            if isinstance(dop1, Dop):  # dop1 is Dop
+            # convert values to multiplicative operators
+            if isinstance(dop1, Dop):
                 if not isinstance(dop2, Mv):
                     dop2 = dop1.Ga.mv(dop2)
-                dop2 = Dop([dop2], [dop1.Ga.Pdop_identity], cmpflg=dop1.cmpflg, ga=dop1.Ga)
-            else:  # dop2 is Dop
+                dop2 = Dop([(dop2, dop1.Ga.Pdop_identity)], cmpflg=dop1.cmpflg, ga=dop1.Ga)
+            elif isinstance(dop2, Dop):
                 if not isinstance(dop1, Mv):
                     dop1 = dop2.Ga.mv(dop1)
-                dop1 = Dop([dop1], [dop2.Ga.Pdop_identity], cmpflg=dop2.cmpflg, ga=dop2.Ga)
+                dop1 = Dop([(dop1, dop2.Ga.Pdop_identity)], cmpflg=dop2.cmpflg, ga=dop2.Ga)
+            else:
+                raise TypeError("Neither argument is a Dop instance")
             return Dop.Add(dop1, dop2)
 
     def __add__(self, dop):

test/test_differential_ops.py

@@ -17,16 +17,22 @@ def test_associativity_and_distributivity(self):
         # check addition distributes
         assert (laplacian + ga.grad) * v == laplacian * v + ga.grad * v != 0
         assert (laplacian + 1234567) * v == laplacian * v + 1234567 * v != 0
+        assert (1234 * ex + ga.grad) * v == 1234 * ex * v + ga.grad * v != 0
         # check subtraction distributes
         assert (laplacian - ga.grad) * v == laplacian * v - ga.grad * v != 0
         assert (laplacian - 1234567) * v == laplacian * v - 1234567 * v != 0
+        assert (1234 * ex - ga.grad) * v == 1234 * ex * v - ga.grad * v != 0
         # check unary subtraction distributes
         assert (-ga.grad) * v == -(ga.grad * v) != 0
         # check division is associative
         assert v * (ga.rgrad / 2) == (v * ga.rgrad) / 2 != 0
         # check multiplication is associative
+        assert (ex * ga.grad) * v == ex * (ga.grad * v) != 0
         assert (20 * ga.grad) * v == 20 * (ga.grad * v) != 0
+        assert v * (ga.rgrad * ex) == (v * ga.rgrad) * ex != 0
         assert v * (ga.rgrad * 20) == (v * ga.rgrad) * 20 != 0
+        assert (laplacian * ga.grad) * v == laplacian * (ga.grad * v) != 0
+        # check wedge is associative
         assert (ex ^ ga.grad) ^ v == ex ^ (ga.grad ^ v) != 0
         assert (20 ^ ga.grad) ^ v == 20 ^ (ga.grad ^ v) != 0
         assert v ^ (ga.rgrad ^ ex) == (v ^ ga.rgrad) ^ ex != 0
@@ -128,3 +134,21 @@ def test_deprecation(self):
         with pytest.warns(DeprecationWarning):
             p = Pdop(None, ga=ga)
         assert p == Pdop({}, ga=ga)
+
+    def test_associativity_and_distributivity(self):
+        coords = x, y, z = symbols('x y z', real=True)
+        ga, ex, ey, ez = Ga.build('e*x|y|z', g=[1, 1, 1], coords=coords)
+        v = ga.mv('v', 'vector', f=True)
+        laplacian = Sdop((ga.grad * ga.grad).terms, ga=ga)
+
+        # check addition distributes
+        assert (laplacian + 20) * v == laplacian * v + 20 * v != 0
+        assert (20 + laplacian) * v == laplacian * v + 20 * v != 0
+        # check subtraction distributes
+        assert (laplacian - 20) * v == laplacian * v - 20 * v != 0
+        assert (20 - laplacian) * v == 20 * v - laplacian * v != 0
+        # check unary subtraction distributes
+        assert (-laplacian) * v == -(laplacian * v) != 0
+        # check multiplication is associative
+        assert (20 * laplacian) * v == 20 * (laplacian * v) != 0
+        assert (laplacian * laplacian) * v == laplacian * (laplacian * v) != 0