Skip to content

Conversation

@krdk01
Copy link

@krdk01 krdk01 commented Oct 3, 2024

#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

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
@langchain4j
Copy link
Collaborator

Hi @thrkrdk, thank you! When does this situation happen? Could you please add some test(s) to ensure the bug is fixed? Thanks!


private float[] weightedAverage(List<float[]> embeddings, List<Integer> weights) {
if (embeddings.isEmpty()) {
throw new RuntimeException("Embeddings list cannot be empty");
Copy link

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?


private float[] weightedAverage(List<float[]> embeddings, List<Integer> weights) {
if (embeddings.isEmpty()) {
throw new RuntimeException("Embeddings list cannot be empty");
Copy link

Choose a reason for hiding this comment

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

Suggested change
throw new RuntimeException("Embeddings list cannot be empty");
throw new IllegalArgumentException("Embeddings list cannot be empty");

@sebastianbogner
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants