Skip to content

Commit 870a6b0

Browse files
Some more optimization
1 parent 411bd33 commit 870a6b0

4 files changed

Lines changed: 57 additions & 65 deletions

File tree

src/smsfusion/_coning_sculling.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import numpy as np
22
from numpy.typing import ArrayLike
33

4-
from smsfusion._vectorops import _cross, _determinant_3_by_3, _inverse_3_by_3
4+
from smsfusion._vectorops import _adjugate_and_det_3_by_3, _cross
55

66

77
class ConingScullingAlg:
@@ -196,15 +196,21 @@ def __init__(
196196
b_f: np.ndarray = np.zeros(3),
197197
bias_alt: bool = False,
198198
):
199-
W_w_det = _determinant_3_by_3(W_w)
200-
W_w_inv = _inverse_3_by_3(W_w, determinant=W_w_det)
199+
adj_W_w, W_w_det = _adjugate_and_det_3_by_3(W_w)
200+
if W_w_det == 0:
201+
raise ValueError("W_w must be invertible")
202+
W_w_inv = adj_W_w / W_w_det
201203
self.cof_W = W_w_inv.T * W_w_det
202204
self.W_star = W_w_inv @ W_f
203205
if bias_alt:
204206
self.b_f_star = b_f
205207
self.b_w_star = b_w
206208
else:
207-
self.b_f_star = _inverse_3_by_3(W_f) @ b_f
209+
adj_W_f, W_f_det = _adjugate_and_det_3_by_3(W_f)
210+
if W_f_det == 0:
211+
raise ValueError("W_f must be invertible.")
212+
W_f_inv = adj_W_f / W_f_det
213+
self.b_f_star = W_f_inv @ b_f
208214
self.b_w_star = W_w_inv @ b_w
209215
self.W_w = W_w
210216
super().__init__(fs)

src/smsfusion/_vectorops.py

Lines changed: 16 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -93,23 +93,29 @@ def _skew_symmetric(a: NDArray[np.float64]) -> NDArray[np.float64]:
9393

9494

9595
@njit # type: ignore[misc]
96-
def _determinant_3_by_3(m: np.ndarray) -> float:
96+
def _adjugate_and_det_3_by_3(
97+
m: NDArray[np.float64],
98+
) -> tuple[NDArray[np.float64], float]:
9799
"""
98-
Calculates and returns the determinant of the matrix
100+
Calculates and returns the adjugate matrix and determinant of the matrix
99101
```python
100102
m = [[a, b, c],
101103
[d, e, f],
102104
[g, h, i]]
103105
```
106+
If the determinant is non-zero, one can calculate the inverse of m as
107+
``inv(m) = adj(m) / det(m)``.
104108
105109
Parameters
106110
----------
107-
m : array-like, shape (3, 3)
108-
The matrix for which to calculate the determinant.
111+
m : numpy.ndarray, shape (3, 3)
112+
The matrix for which to calculate the adjugate and determinant.
109113
110114
Returns
111115
-------
112-
det : float
116+
adj_m : numpy.ndarray, shape (3, 3)
117+
The adjugate of the input matrix m.
118+
det_m : float
113119
The determinant of the input matrix m.
114120
"""
115121
a, b, c = m[0]
@@ -118,49 +124,13 @@ def _determinant_3_by_3(m: np.ndarray) -> float:
118124
A = e * i - f * h
119125
B = -(d * i - f * g)
120126
C = d * h - e * g
121-
122-
return a * A + b * B + c * C
123-
124-
125-
@njit # type: ignore[misc]
126-
def _inverse_3_by_3(m: np.ndarray, determinant: float | None = None):
127-
"""
128-
Calculates and returns the inverse of the matrix
129-
```python
130-
m = [[a, b, c],
131-
[d, e, f],
132-
[g, h, i]]
133-
```
134-
135-
Parameters
136-
----------
137-
m : array-like, shape (3, 3)
138-
The matrix to be inverted.
139-
determinant : float, optional
140-
The determinant of the input matrix m. If not provided, it will be calculated
141-
internally.
142-
143-
Returns
144-
-------
145-
inv_m : ndarray, shape (3, 3)
146-
The inverse of the input matrix m.
147-
"""
148-
if m.shape != (3, 3):
149-
raise ValueError("Input matrix must be 3x3.")
150-
a, b, c = m[0]
151-
d, e, f = m[1]
152-
g, h, i = m[2]
153-
A = e * i - f * h
154-
B = -(d * i - f * g)
155-
C = d * h - e * g
156127
D = -(b * i - c * h)
157128
E = a * i - c * g
158129
F = -(a * h - b * g)
159130
G = b * f - c * e
160131
H = -(a * f - c * d)
161-
I = a * e - b * d
162-
if determinant is None:
163-
determinant = _determinant_3_by_3(m)
164-
if determinant == 0:
165-
raise ValueError("Input matrix is singular and cannot be inverted.")
166-
return np.array([[A, D, G], [B, E, H], [C, F, I]]) / determinant
132+
I_ = a * e - b * d
133+
# Equations fetched from https://en.wikipedia.org/wiki/Invertible_matrix#Inversion_of_3_%C3%97_3_matrices
134+
adj_m = np.array([[A, D, G], [B, E, H], [C, F, I_]])
135+
det_m = a * A + b * B + c * C
136+
return adj_m, det_m

tests/test_coning_sculling.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,3 +389,28 @@ def test_verify_calibration(self, data_ag, data_dtheta_dvel):
389389
np.testing.assert_allclose(dvel_calibrated_alt, dvel_true, atol=1e-8)
390390
np.testing.assert_allclose(dtheta_naive, dtheta_true, atol=1e-8)
391391
np.testing.assert_allclose(dvel_naive, dvel_true, atol=1e-8)
392+
393+
@pytest.mark.parametrize(
394+
"w_singular, f_singular", [(True, False), (False, True), (True, True)]
395+
)
396+
def test_raises_singular_matrix(self, w_singular, f_singular):
397+
fs = 100.0
398+
if w_singular:
399+
W_w = np.array([[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 0.0]])
400+
else:
401+
W_w = np.eye(3)
402+
if f_singular:
403+
W_f = np.array([[1.0, 0.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 0.0]])
404+
else:
405+
W_f = np.eye(3)
406+
b_w = np.zeros(3)
407+
b_f = np.zeros(3)
408+
with pytest.raises(ValueError, match="must be invertible"):
409+
sf.ConingScullingAlgCalibrated(
410+
fs, W_w=W_w, W_f=W_f, b_w=b_w, b_f=b_f, bias_alt=False
411+
)
412+
if w_singular:
413+
with pytest.raises(ValueError, match="must be invertible"):
414+
sf.ConingScullingAlgCalibrated(
415+
fs, W_w=W_w, W_f=W_f, b_w=b_w, b_f=b_f, bias_alt=True
416+
)

tests/test_vectorops.py

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -72,23 +72,14 @@ def test__skew_symmetric():
7272
np.testing.assert_array_equal(out, expected)
7373

7474

75-
def test__inverse_and_determinant_3_by_3():
76-
"""Test matrix inversion and determinant against numpy"""
75+
def test__adjugate_and_det_3_by_3():
76+
"""Test matrix inversion against numpy"""
7777
rng = np.random.default_rng(42)
7878
for _ in range(5):
7979
m = rng.random((3, 3))
80-
det = _vectorops._determinant_3_by_3(m)
81-
inv = _vectorops._inverse_3_by_3(m)
82-
inv_with_det = _vectorops._inverse_3_by_3(m, determinant=det)
83-
det_expected = np.linalg.det(m)
84-
inv_expected = np.linalg.inv(m)
85-
np.testing.assert_allclose(det, det_expected, rtol=1e-12, atol=1e-12)
86-
np.testing.assert_allclose(inv, inv_expected, rtol=1e-12, atol=1e-12)
87-
np.testing.assert_allclose(inv_with_det, inv_expected, rtol=1e-12, atol=1e-12)
8880

81+
adj, det = _vectorops._adjugate_and_det_3_by_3(m)
82+
inv = adj / det
8983

90-
def test__inverse_3_by_3_singular():
91-
"""Test that the inverse function raises an error for singular matrices"""
92-
singular_matrix = np.array([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
93-
with pytest.raises(ValueError):
94-
_vectorops._inverse_3_by_3(singular_matrix)
84+
inv_expected = np.linalg.inv(m)
85+
np.testing.assert_allclose(inv, inv_expected, rtol=1e-12, atol=1e-12)

0 commit comments

Comments
 (0)