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: optimize string comparisons to reduce llm calls #216

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sushruth2003
Copy link
Contributor

@sushruth2003 sushruth2003 commented Nov 29, 2024

Fixes #125
Making some simple fixes to bypass llm calls in the case of easily solved string comparisons.

@@ -74,6 +85,22 @@ def compare_pair(
):
return True, 0

# For each key that exists in both items, try basic string matching
common_keys = set(item1.keys()) & set(item2.keys())
Copy link
Collaborator

Choose a reason for hiding this comment

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

One worry I have here is that it won't be performant...LLM calls are not performant but at least they can be multithreaded. Do you have an idea about the performance characteristics of slugify?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already check for case-insensitive exact matches in lines 80-86 (in the edited file), so I wonder what the marginal benefit of using slugify is

Copy link
Collaborator

Choose a reason for hiding this comment

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

tagging @redhog to give feedback on this, since they raised the issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, I'll see if I can profile on some of the test cases with it on and off, but most of what I can read online says that the slugify library is pretty efficient.

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.

Rresolve/Link resolve: Solve simple cases fast
2 participants