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

LSE Omnibus embedding #835

Merged
merged 25 commits into from Sep 15, 2021
Merged

LSE Omnibus embedding #835

merged 25 commits into from Sep 15, 2021

Conversation

nicaurvi
Copy link
Contributor

@nicaurvi nicaurvi commented Sep 13, 2021

  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
  • Have you built the documentation (reference and/or tutorial) and verified the generated documentation is appropriate?

Reference Issues/PRs

What does this implement/fix? Briefly explain your changes.

Add ability to create LSE omnibus embeddings.

Any other comments?

Based off of @zeou1 PR

@nicaurvi nicaurvi mentioned this pull request Sep 13, 2021
@nicaurvi nicaurvi changed the title Lse omni LSE Omnibus embedding Sep 13, 2021
@@ -145,6 +168,10 @@ class OmnibusEmbed(BaseEmbedMulti):
Only applicable for ``algorithm="randomized"``; allows you to seed the
randomized svd solver for deterministic, albeit pseudo-randomized behavior.

lse : bool, optional (default False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled this from the earlier LSE Omni PR -- Is this the api we can all agree on or does someone have other suggestions?

In topologic, I believe we used an enum here (ase or lse, default ase.) Other devs may prefer a magic string but I'd prefer not.

We can also leave this as is and change it in the future if it turns out we want to support something other than ase/lse.

Lbar = (L1 + L2) / 2

omni = OmnibusEmbed(n_components=3, diag_aug=diag_aug, lse=True)
OmniBar = compute_bar(omni.fit_transform([csr_matrix(L1), csr_matrix(L2)]))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works with sparse.

@nicaurvi nicaurvi marked this pull request as ready for review September 13, 2021 18:11
graspologic/embed/omni.py Outdated Show resolved Hide resolved
@bdpedigo
Copy link
Collaborator

implementation looks fine, my continued queasyness is just that I don't have a simulation / data example that I can show someone that says when LSE omni would be preferred

@daxpryce
Copy link
Contributor

daxpryce commented Sep 13, 2021

implementation looks fine, my continued queasyness is just that I don't have a simulation / data example that I can show someone that says when LSE omni would be preferred

what level of queasiness are we talking about? like, we're currently using it to measure fluidity in our viva insights product to calculate https://docs.microsoft.com/en-us/workplace-analytics/azure-templates/ona-metric-calculations#fluidity

or are you more wondering if there's an academic paper on it? because I don't think we have that

edit: jlar has an entire slide deck he can go through with you if that would help.

@nicaurvi nicaurvi self-assigned this Sep 14, 2021
@nicaurvi nicaurvi linked an issue Sep 14, 2021 that may be closed by this pull request
@bdpedigo
Copy link
Collaborator

what level of queasiness are we talking about?

not extremely, just think it would be good to have a public-facing reason for using this stuff as there is no associated publication, as you mention

@daxpryce
Copy link
Contributor

should @nyecarr reference that page in the docs as well?

@nicaurvi nicaurvi merged commit d1b278e into dev Sep 15, 2021
@bdpedigo bdpedigo deleted the lse-omni branch September 15, 2021 21:47
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 LSE-OMNI
4 participants