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

OpenAiEmbeddingModel failes to instantiate unless embedding-model-dimensions.properties is manually added to class path #1709

Open
russdanner opened this issue Nov 10, 2024 · 2 comments
Labels

Comments

@russdanner
Copy link

Please do a quick search on GitHub issues first, there might be already a duplicate issue for the one you are about to create.
If the bug is trivial, just go ahead and create the issue. Otherwise, please take a few moments and fill in the following sections:

Bug description
org.springframework.ai.embedding.AbstractEmbeddingModel function loadKnownModelDimensions is called statically to load a properties file classpath:/embedding/embedding-model-dimensions.properties that is not guaranteed to be in the classpath.

It should be possible to:

  • Run the system on defaults without externally placing the properties file
  • Change the location of the resource
  • Override how this resource is defined.

Environment
SpringAI SNAPSHOT-1.0.0

Steps to reproduce
Attempt to instantiate OpenAiEmbeddingModel without manually placing the properties file in the class path.

Expected behavior
N/A

Minimal Complete Reproducible example

def embeddingOptions = OpenAiEmbeddingOptions.builder().withModel("text-embedding-ada-002").build()
def embeddingModel = new OpenAiEmbeddingModel(openAiApi, MetadataMode.EMBED, embeddingOptions)
@asaikali asaikali added vector store bug Something isn't working labels Nov 10, 2024
@markpollack
Copy link
Member

Perhaps this is related to how kotlin consumes the jars, but the file mentioned is in the resources directory of spring-ai-core so it should be resolved as your application has a dependency on that module.

The request to change the location or override is another feature that is worth to consider, but separate from the classpath issue.

Can you clarify?

Another option is that we just get rid of this file completely, then we make 1 call to the model to get the embedding value.

Balancing the cost of maintaining this file vs making an additional call to the model, I'd go with getting rid of the file.

@russdanner
Copy link
Author

russdanner commented Nov 14, 2024

Hi @markpollack

I agree that the classpath and initialization concerns are different but related.

With respect to the properties file: Assuming it's kept as a requirement, the request is to make sure there's a default version that ships with the jars so that you can import the Spring Jar and load the class without supplying your own. Getting rid of the properties file also works :)

Here's a bit of weight to put in the scale on the side of eliminating the file:

Kotlin, Groovy, and other scripting environments allow you to deploy a script that dynamically loads a dependency like Spring AI.

@GrabResolver(name='custom', root='https://repo.spring.io/snapshot', m2Compatible='true')
@Grab(group='org.springframework.ai', module='spring-ai-core', version='1.0.0-SNAPSHOT', initClass=false)
@Grab(group='org.springframework.ai', module='spring-ai-openai', version='1.0.0-SNAPSHOT', initClass=false)

In this context, you are deploying a simple script as a file rather than a jar. In some cases, there is no opportunity to deploy a properties file into the classpath unless you create your own jar, deploy that jar to a private repository, and include it similarly to the jars above -- which is too much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants