Repository: sympy/sympy
Pull request: Removed lazy calculation and storage of norm
#### References to other Issues or PRs
Fixes  #25254

#### Release Notes

<!-- BEGIN RELEASE NOTES -->
NO ENTRY
<!-- END RELEASE NOTES -->


Problem statement:
Quaternions in version 12
Dear friends,
I have detected a problem when multiplying Quaternions using the * to do the product between two.  It also fails when using q.mul(p).  In my case, one of the quaternions was obtained by the "from Axis angle". This code gives me problems:

`p= Quaternion(1,0,1,0);
q = Quaternion.from_axis_angle((1,1,1), 3*np.pi/4);
qi =q.inverse();
p1= q*p*qi `

The error is not there for any previous versions 11.x.

Many thanks for considering my request.

Hints from issue discussion:
CC @evbernardes 

```python
In [3]: p= Quaternion(1,0,1,0); q = Quaternion.from_axis_angle((1,1,1), 3*np.pi/4); qi =q.inverse()

In [4]: q*p
---------------------------------------------------------------------------
ValueError: Incompatible value for norm.
```
Hello @singladur. I think the bug itself comes from `3*np.pi/4` line, since the used value or `pi` is the numerical value from `numpy`. Changing it to `3*sympy.pi/4` would solve it.

This shouldn't be a big deal though though, so I'd say it's more of a **design fault from my part**.

To remove this problem from the `Sympy` source, this:
```python
def norm(self):
    if self._norm is None:  # check if norm is pre-defined
        q = self
        self._norm = sqrt(trigsimp(q.a**2 + q.b**2 + q.c**2 + q.d**2))

    return self._norm
```
Can be changed to this:

```python
def norm(self):
    if self._norm is None:  # check if norm is pre-defined
        q = self
        return sqrt(trigsimp(q.a**2 + q.b**2 + q.c**2 + q.d**2))

    return self._norm
```
Since removing this lazy calculation and storage of the norm value would avoid forcing the `norm` attritube in every `Quaternion`, except when explicitly asked.

@oscarbenjamin If this is okay for you, I made a PR with this. Except if you plan on removing the norm system entirely, like we were discussing some months ago.

Commit message: fixed test

Patch to review:
diff --git a/sympy/algebras/quaternion.py b/sympy/algebras/quaternion.py
index d0a8915ee9c0..42dd989ec648 100644
--- a/sympy/algebras/quaternion.py
+++ b/sympy/algebras/quaternion.py
@@ -884,7 +884,7 @@ def norm(self):
             q = self
             # trigsimp is used to simplify sin(x)^2 + cos(x)^2 (these terms
             # arise when from_axis_angle is used).
-            self._norm = sqrt(trigsimp(q.a**2 + q.b**2 + q.c**2 + q.d**2))
+            return sqrt(trigsimp(q.a**2 + q.b**2 + q.c**2 + q.d**2))
 
         return self._norm
 
diff --git a/sympy/algebras/tests/test_quaternion.py b/sympy/algebras/tests/test_quaternion.py
index edf65ec18e08..4896a0e73e29 100644
--- a/sympy/algebras/tests/test_quaternion.py
+++ b/sympy/algebras/tests/test_quaternion.py
@@ -13,6 +13,7 @@
 from sympy.simplify.trigsimp import trigsimp
 from sympy.algebras.quaternion import Quaternion
 from sympy.testing.pytest import raises
+import numpy as np
 from itertools import permutations, product
 
 w, x, y, z = symbols('w:z')
@@ -46,6 +47,18 @@ def test_quaternion_construction_norm():
     assert (q1 * q3).norm() == q1.norm()
 
 
+def test_issue_25254():
+    # calculating the inverse cached the norm which caused problems
+    # when multiplying
+    p = Quaternion(1, 0, 1, 0)
+    q = Quaternion.from_axis_angle((1,1,1), 3 * np.pi/4)
+    qi = q.inverse()  # this operation cached the norm
+    try:
+        test = q * p * qi
+    except ValueError as exc:
+        assert False, f"'10 / 5' raised an exception {exc}"
+
+
 def test_to_and_from_Matrix():
     q = Quaternion(w, x, y, z)
     q_full = Quaternion.from_Matrix(q.to_Matrix())


Code review comments:
diff_hunk: @@ -46,6 +47,18 @@ def test_quaternion_construction_norm():
     assert (q1 * q3).norm() == q1.norm()
 
 
+def test_issue_25254():
+    # calculating the inverse cached the norm which caused problems
+    # when multiplying
+    p = Quaternion(1, 0, 1, 0)
+    q = Quaternion.from_axis_angle((1,1,1), 3 * np.pi/4)
path: sympy/algebras/tests/test_quaternion.py
text: @user1:
We can test with `math.pi` constant to remove dependency with numpy.