Add new matrix reconditioning approach. - #200
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
|
Also removing some code that added very little but raised a lot of warnings each run. |
Harry-Rich
left a comment
Harry-Rich
left a comment
There was a problem hiding this comment.
Can't really comment on method, happy to approve, see what you think about comments
| def test_eigenvalue_clipping(self): | ||
| matrix = np.random.random((100, 100)) + 100 | ||
| reconditioned_matrix = eigenvalue_clipping(matrix) | ||
| assert not np.allclose(matrix, reconditioned_matrix) |
There was a problem hiding this comment.
This only really tests the dimensionality right, should we test that is it returning the correct values?
There was a problem hiding this comment.
Yes, I think once we have investigated the different clipping approaches a real test is a good idea. This is just checking that something has changed.
| :param a: input array | ||
| :return: element wise maximum of (a, 0) | ||
| """ | ||
| return np.maximum(a, np.zeros_like(a)) |
There was a problem hiding this comment.
np.maximum(a, 0.0)np maximum maintains the array size.
| new_eigenvalues = np.copy(eigenvalues) | ||
| new_eigenvalues[new_eigenvalues < lambda_plus] = lambda_minus | ||
|
|
||
| new_corr = (eigenthings.eigenvectors @ np.diag(new_eigenvalues) @ np.linalg.inv(eigenthings.eigenvectors)).real |
There was a problem hiding this comment.
maybe define eigenvectors similarly to eigenvalues for consistency?
| values=cov_nearest( | ||
| minimum_eigenvalue_method(cov[self.diff_regime :, self.diff_regime :], self._cond_max) | ||
| ), | ||
| unit=self.dg['da'].unit ** 2, |
There was a problem hiding this comment.
Don't find dg or da naming very intuitive, but this is not really a new change, perhaps something to think about (I am sure there was some rationale)
2 participants
This is very much an experimental approach that I am hoping to have a student investigate in detail over summer. Hence this does not affect the standard operation of kinisi.
Just need to write test for this.