Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added basic code to calculate the magnitude function #89

Closed
wants to merge 4 commits into from

Conversation

lizliz
Copy link
Collaborator

@lizliz lizliz commented Feb 29, 2024

Ok, first pass on adding in the code I had. I'm sure I'm forgetting something additional to do, particularly with documentation...... I tried to make sure the function has lots of description. Where else do I want to put this? More documentation somewhere?

@lizliz lizliz linked an issue Feb 29, 2024 that may be closed by this pull request
@Pseudomanifold
Copy link

Sorry, don't know how to properly add to this request...but here's some code that calculates magnitude using a Cholesky decomposition.

from scipy.linalg import cho_factor
from scipy.linalg import solve_triangular

[...]

M = np.exp(-D)
c, lower = cho_factor(M)
x = solve_triangular(c, np.ones(M.shape[0]), trans=1)

return x.T @ x

The idea is that the similarity matrix $\zeta$ is positive definite (under nice conditions), so it affords a Cholesky decomposition, which factorises as $L L^T$ for $L$ a lower-triangular matrix. Magnitude can be written as $\textrm{Mag}(X) = \mathbb{1}^T \zeta^{-1} \mathbb{1}$, i.e. a quadratic form (since we are taking row sums). Thus, calculating magnitude is equivalent to calculating $x^T x$ with $x$ defined by $Lx = \mathbb{1}$.

(We have described this a little bit in Appendix B of a recent preprint)

Hope that helps!

@barnesd8
Copy link
Collaborator

@Pseudomanifold does that code need to be included in this merge as an additional function?

@lizliz do you want this approved without a unit test? We would also want to increment the version in the pyproject.toml so it can auto deploy to pypi.

@lizliz
Copy link
Collaborator Author

lizliz commented Feb 29, 2024

Don't approve yet, I should probably put this back to a draft while I finish doing the rest of the cleanup

@barnesd8
Copy link
Collaborator

@lizliz this also has to go into the test branch before master too

@Pseudomanifold
Copy link

@barnesd8 I think it could be included as an alternative calculation function

@barnesd8
Copy link
Collaborator

included in other pull request

@barnesd8 barnesd8 closed this Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add magnitude function
3 participants