Skip to content

Conversation

@thwu1
Copy link
Collaborator

@thwu1 thwu1 commented Jul 9, 2024

#9 I added precompution for collapsing three linear transformations without modifying the loading part. @iojw Do you want to do a speed test?

@thwu1 thwu1 changed the title Optimization for MF: Precompute collapsed linear transformation Optimization for MF Jul 9, 2024
@thwu1 thwu1 requested a review from iojw July 9, 2024 18:46
Copy link
Collaborator

@iojw iojw left a comment

Choose a reason for hiding this comment

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

Thank you @thwu1! This will be an amazing improvement for MF - some comments.

use_proj,
dim=128,
num_models=64,
text_dim=768,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we automatically set the text dim based on the selected embeddings?

text_dim=768,
num_classes=1,
use_proj=True,
embedding_model="all-mpnet-base-v2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe better to use OpenAI's embeddings by default to preserve our existing behavior? I'll add some docs to describe the different options here.

num_classes=1,
use_proj=True,
collapse_linear=False,
embedding_model="all-mpnet-base-v2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better if we didn't set any default args here and specify default args only in routers.py so that it's easier to keep track of router configs.

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.

3 participants