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

does low rank approximation violate gauge conditions? #132

Open
wsdewitt opened this issue Sep 28, 2021 · 1 comment · May be fixed by #137
Open

does low rank approximation violate gauge conditions? #132

wsdewitt opened this issue Sep 28, 2021 · 1 comment · May be fixed by #137
Assignees

Comments

@wsdewitt
Copy link
Contributor

We have the code below in analysis.py for low rank approximation of the beta matrix. A couple points:

  • This looks like it would violate our WT gauge condition, and it might take a little more thinking to get these to be mutually consistent.
  • We should probably have a test for this (testing if gauge is still maintained after low rank approximation).
  • I suspect it may not even be possible to make these compatible, given our zeroing of unseen mutations. One of the main advantages of low rank approximations is to help "fill in" parts of the matrix that we haven't observed by postulating shared profiles, but this isn't possible if our gauge mandates unobserved entries are zero. We need to think harder about this.
  • Picky coding point: this is the only remaining part of analysis.py that has conditions on the presence of under-the-hood model attributes like model_bind and model_stab. As we have done with gauge fixing, this suggests that the low rank approximation operation should be a model method (enforced by the abstract base class), and we can then entirely avoid model introspection in analysis.py.

if beta_rank is not None:
# procedure for 2D models.
if hasattr(self.model, "model_bind") and hasattr(
self.model, "model_stab"
):
num_latent_dims = self.model.model_bind.latent_dim
for latent_dim in range(num_latent_dims):
_make_beta_matrix_low_rank(
self.model.model_bind,
latent_dim,
beta_rank,
self.val_data.wtseq,
self.val_data.alphabet,
)
_make_beta_matrix_low_rank(
self.model.model_stab,
latent_dim,
beta_rank,
self.val_data.wtseq,
self.val_data.alphabet,
)
else:
num_latent_dims = self.model.latent_dim
for latent_dim in range(num_latent_dims):
_make_beta_matrix_low_rank(
self.model,
latent_dim,
beta_rank,
self.val_data.wtseq,
self.val_data.alphabet,
)

@zorian15
Copy link
Collaborator

Ah thanks for raising this @wsdewitt! I can add this to my list!

@zorian15 zorian15 self-assigned this Sep 28, 2021
@zorian15 zorian15 linked a pull request Oct 9, 2021 that will close this issue
7 tasks
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 a pull request may close this issue.

2 participants