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

Improve node distance reranker speed #107

Merged
merged 3 commits into from
Sep 12, 2024
Merged

Conversation

prasmussen15
Copy link
Collaborator

@prasmussen15 prasmussen15 commented Sep 12, 2024

🚀 This description was created by Ellipsis for commit 71920a9

perf: optimize node distance reranker and update podcast example

Summary:

Optimize node_distance_reranker(), update podcast_runner.py, and clean up test_graphiti_int.py.

Key points:

  • Optimize node_distance_reranker() in search_utils.py using asyncio.gather for parallel execution.
  • Update podcast_runner.py to process messages from 11 to 127 when use_bulk is False.
  • Remove unused test code in test_graphiti_int.py related to build_communities() and initial search query.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to a903094 in 1 minute and 41 seconds

More details
  • Looked at 100 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. examples/podcast/podcast_runner.py:66
  • Draft comment:
    Increasing the range from 3:14 to 3:130 may lead to performance issues if the number of messages is large. Consider the impact on execution time.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change in the range of messages from 3:14 to 3:130 in the loop might lead to performance issues if the number of messages is large. This should be noted as it could affect the execution time significantly.

Workflow ID: wflow_Ph4He4BILOZuxENy


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

graphiti_core/search/search_utils.py Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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! Incremental review on de09ab7 in 1 minute and 23 seconds

More details
  • Looked at 149 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. graphiti_core/search/search_utils.py:38
  • Draft comment:
    The indentation of the function parameters is inconsistent with PEP 8 style guidelines. Consider aligning them with the opening parenthesis for better readability. This issue is also present in other functions like entity_similarity_search, entity_fulltext_search, edge_fulltext_search, hybrid_node_search, get_relevant_nodes, and get_relevant_edges.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The indentation of the function parameters is inconsistent with PEP 8 style guidelines. This is a minor style issue but should be corrected for consistency and readability.
2. graphiti_core/search/search_utils.py:517
  • Draft comment:
    Consider using a more descriptive variable name than records for the result of result[0] to improve code clarity. Additionally, the logic for setting distance could be simplified by directly assigning record['score'] when record is not None.
  • Reason this comment was not posted:
    Confidence changes required: 30%
    The node_distance_reranker function uses asyncio.gather to execute queries in parallel, which is a good optimization for performance. However, the logic for handling the results could be improved for clarity and efficiency.

Workflow ID: wflow_gTCBuPVbWSr04Gmg


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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! Incremental review on 71920a9 in 1 minute and 0 seconds

More details
  • Looked at 151 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_rA51zzhGM00gHrRq


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@prasmussen15 prasmussen15 merged commit 85cf8e5 into main Sep 12, 2024
6 checks passed
@prasmussen15 prasmussen15 deleted the update-node-distance-search branch September 12, 2024 15:23
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants