-
Notifications
You must be signed in to change notification settings - Fork 43
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
#923 fix for IndexOutOfBoundsException #41
base: main
Are you sure you want to change the base?
#923 fix for IndexOutOfBoundsException #41
Conversation
IndexOutOfBoundsException execption is fixed. Add validation to ensure embeddings list is not empty. changes: -- added a check for an empty "embeddings" -- Throws RuntimeException if the embeddings list empty
Hi @thrkrdk, thank you! When does this situation happen? Could you please add some test(s) to ensure the bug is fixed? Thanks! |
@@ -197,6 +197,10 @@ private static float[] meanPool(float[][] vectors) { | |||
} | |||
|
|||
private float[] weightedAverage(List<float[]> embeddings, List<Integer> weights) { | |||
if (embeddings.isEmpty()) { | |||
throw new RuntimeException("Embeddings list cannot be empty"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest using more specific IllegalArgumentException
?
@@ -197,6 +197,10 @@ private static float[] meanPool(float[][] vectors) { | |||
} | |||
|
|||
private float[] weightedAverage(List<float[]> embeddings, List<Integer> weights) { | |||
if (embeddings.isEmpty()) { | |||
throw new RuntimeException("Embeddings list cannot be empty"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new RuntimeException("Embeddings list cannot be empty"); | |
throw new IllegalArgumentException("Embeddings list cannot be empty"); |
Hey guys! The last couple of days I was tinkering around with LLM stuff and followed some examples from a Java magazine and ended up with the same error while trying to embed a PDF file. I uploaded my code to Github: https://github.com/sebastianbogner/llm-test You can reproduce it by running Chroma via the docker-compose file and then just run the main method in HelloChroma.java. I'm not sure if the suggested fix will really solve the problem. Shouldn't the method weightedAverage() just return an empty float array as suggested in this PR? Regards |
#923 - IndexOutOfBoundsException execption is fixed. Add validation to ensure embeddings list is not empty. changes:
-- added a check for an empty "embeddings"
-- Throws RuntimeException if the embeddings list empty