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

Add Retry Events #8053

Merged
merged 21 commits into from
Jul 27, 2023
Merged

Add Retry Events #8053

merged 21 commits into from
Jul 27, 2023

Conversation

hinthornw
Copy link
Collaborator

@hinthornw hinthornw commented Jul 21, 2023

image

@vercel
Copy link

vercel bot commented Jul 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 27, 2023 6:21pm

@dosubot dosubot bot added the 🤖:improvement Medium size change to existing code to handle new use-cases label Jul 21, 2023
@leo-gan
Copy link
Collaborator

leo-gan commented Jul 21, 2023

Unit tests would be nice for this important code.

retry_d["outcome"] = "failed"
exception = retry_state.outcome.exception()
retry_d["exception"] = str(exception)
retry_d["exception_cls"] = exception.__class__.__name__
Copy link
Collaborator

Choose a reason for hiding this comment

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

cls is a python thing, can we do something more cross-platform-y?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

exception_type perhaps?

@hinthornw hinthornw requested review from nfcampos and baskaryan July 24, 2023 22:01
@baskaryan
Copy link
Collaborator

ik jinachat, cohere, and vertexai (and probs more) have v similar retry logic, should we add there, too? or hold off for sep pr?

@hinthornw
Copy link
Collaborator Author

I think probably separate PR. I'll fix the merge conflicts though

ruoccofabrizio and others added 5 commits July 26, 2023 19:04
Description: Adding support for custom index and scoring profile support
in Azure Cognitive Search
@hwchase17

---------

Co-authored-by: Bagatur <baskaryan@gmail.com>
Optimizing important numerical code and making it run faster.

Performance went up by 1.48x (148%). Runtime went down from 138715us to
56020us

Optimization explanation:

The `cosine_similarity_top_k` function is where we made the most
significant optimizations.
Instead of sorting the entire score_array which needs considering all
elements, `np.argpartition` is utilized to find the top_k largest scores
indices, this operation has a time complexity of O(n), higher
performance than sorting. Remember, `np.argpartition` doesn't guarantee
the order of the values. So we need to use argsort() to get the indices
that would sort our top-k values after partitioning, which is much more
efficient because it only sorts the top-K elements, not the entire
array. Then to get the row and column indices of sorted top_k scores in
the original score array, we use `np.unravel_index`. This operation is
more efficient and cleaner than a list comprehension.

The code has been tested for correctness by running the following
snippet on both the original function and the optimized function and
averaged over 5 times.
```
def test_cosine_similarity_top_k_large_matrices():
    X = np.random.rand(1000, 1000)
    Y = np.random.rand(1000, 1000)
    top_k = 100
    score_threshold = 0.5
    gc.disable()
    counter = time.perf_counter_ns()
    return_value = cosine_similarity_top_k(X, Y, top_k, score_threshold)
    duration = time.perf_counter_ns() - counter
    gc.enable()
```

@hwaking @hwchase17 @jerwelborn 

Unit tests pass, I also generated more regression tests which all
passed.
Still retain:
- Comparison Examples
- Data + QA walkthrough
- QA (but really minimize it)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants