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

feat: Adding embedding cache for gdc case #76

Merged
merged 4 commits into from
Jul 25, 2024
Merged

Conversation

EduardoPena
Copy link
Collaborator

This is related to #51 issue. This feature allows fast interactions with the user, for example, from ~30s to ~1.5s when matching the 740 columns of GDC using the CL approach. We might want to add support for arbitrary data sets as well, but I am not sure.

@aecio aecio requested a review from EdenWuyifan July 22, 2024 22:52
@aecio
Copy link
Member

aecio commented Jul 24, 2024

@EduardoPena Just tested and it seems to be working fine here. I think my last concern now is what happens when we update the model. We should probably include the model name as a subdirectory in the cache file path so that we don't reuse the cached embedding of a previous model.

@aecio aecio changed the title Adding embedding cache for gdc case feat: Adding embedding cache for gdc case Jul 24, 2024
@EduardoPena
Copy link
Collaborator Author

@aecio , the last commit integrates the model name as a parameter for the cache. Let me know if we are ready to merge into devel.

Copy link
Member

@aecio aecio left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@aecio
Copy link
Member

aecio commented Jul 25, 2024

When merging, I would use the squash option to merge everything in a single clean commit.

@EduardoPena EduardoPena merged commit 0f5a164 into devel Jul 25, 2024
10 checks passed
@roquelopez roquelopez deleted the caching_gdc_embedding branch July 25, 2024 18:41
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