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

Default Embedding Function for JS #1382

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Default Embedding Function for JS #1382

merged 3 commits into from
Jan 5, 2024

Conversation

jeffchuber
Copy link
Contributor

@jeffchuber jeffchuber commented Nov 10, 2023

End dx: npm install chromadb chromadb-default-embed

chromadb-default-embed is a fork of @xenova/transfomers to maintain stability


Motivation

  • good defaults are good DX
  • good defaults lower the barrier to getting started
  • currently JS usage is gated on having an API key (seems bad)

npm install --save chromadb @xenova/transformers

  • We want to use the same EF as python - all-MiniLM-L6-v2
  • We want to keep our default package size small (currently 4.5mb)
  • We want a happy path for devs getting started - they shouldnt need to create any accounts or get API keys
  • @xenova/transformers is great, but it's huge - ~250MB! - so we can't by default bundle it
  • To have a happy path, but keep the bundle size small - we just ask users to run npm install --save chromadb @xenova/transformers to install chroma. we can add a comment like // optional default embedding function

I also evaluated https://github.com/visheratin/web-ai - which is small (~8MB), but I dont think it supports this model yet? (thought potentially possible) and https://github.com/microsoft/onnxruntime/tree/main, which is also massive (over 100MB).

I confirmed that if you just install chromadb and pass OpenAIEmbeddingFunction (or other) - it doesn't complain or yell at you that you have a missing dep. If you true to use the default and don't have @xenova/transformers installed, it will tell you to use it.

Todo

  • test no require warnings in nextjs - this has been an issue in the past

Thoughts about this DX?

Copy link

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readbility, Modularity, Intuitiveness)

@HammadB
Copy link
Collaborator

HammadB commented Nov 11, 2023

Just FYI both web-ai and transformers.js use ONNX runtime under the hood.

@atroyn
Copy link
Contributor

atroyn commented Nov 11, 2023

Echoing Hammad, and as we discussed in person, my preference here would be for us to bundle the same onnx model we ship with python into the JS client, using a framework which supports ONNX models, rather than asking a user to pull in a separate dependency (250MB worth) if they want to use our default.

@jeffchuber
Copy link
Contributor Author

@atroyn ok we will need to see if that is possible. even the MSFT onnx-runtime which I assume is pretty narrow is very heavy.

@atroyn
Copy link
Contributor

atroyn commented Nov 11, 2023

The MSFT onnx runtime appears to be only ~3.65MB? https://www.npmjs.com/package/onnxjs

@jeffchuber
Copy link
Contributor Author

without a lot more surgery (rolling our own solution onnx-runtime) I think that this remains the best path forward. It is pretty common in JS install workflows to have the user install multiple things. eg nextjs does npm install next@latest react@latest react-dom@latest

onnxjs is deprecated, https://www.npmjs.com/package/onnxruntime-node is the new replacement and it's 100MB.

The other alternative would be to bundle it in chromadb and then break out a separate lighter-weight chromadb-client like we do in python. But I don't like this since chromadb in JS can not "stand-alone" like it does in python.

@xenova
Copy link
Contributor

xenova commented Nov 23, 2023

@xenova/transformers is great, but it's huge - ~250MB! - so we can't by default bundle

Remember that models you run are cached in .cache by default, so that is most likely the explanation for the large size.

The unpacked size is "only" 45MB, and it is because we also include the .wasm files in the dist folder.
image

In contrast, other libraries like web-ai (which is split into @visheratin/web-ai and @visheratin/web-ai-node) has a very small unpacked size because it does not distribute the WASM files (accounting for ~40MB). The transformers.js library itself, when built and minified, is only around 800KB (700KB of which is onnxruntime-web).

However, you do bring up a valid point about whether we even need to distribute the WASM files. The original reason for this was that certain versions of ORT weren't very stable, and due to different WASM files, would break if they were updated. Now, however, we have frozen the version, so this probably isn't an issue anymore.

@jeffchuber
Copy link
Contributor Author

@xenova im not sure how i got this wrong, i can confirm @xenova/transformers is about 45mb currently. It would be great if it were smaller - is that a possibility? it would be really great to bundle this by default with chroma's NPM client giving a seamless getting started experience for JS devs.

@jeffchuber
Copy link
Contributor Author

this is how im thinking of updating the website - it would be super slick to have a default EF!

Screenshot 2023-12-19 at 12 43 27 PM

@jeffchuber
Copy link
Contributor Author

Another idea here is to do npx create-chroma-app@latest like https://nextjs.org/docs/getting-started/installation -- that would install all the requirements and a basic app structure. I like this idea the best I think.

Copy link
Collaborator

@HammadB HammadB left a comment

Choose a reason for hiding this comment

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

This makes sense to me!

@jeffchuber jeffchuber merged commit fca3426 into main Jan 5, 2024
98 checks passed
@jeffchuber jeffchuber deleted the feat/js-default-ef branch January 5, 2024 06:30
@Cinemacloud
Copy link

how do you use these in js? I tried: import { embeddingFunction } from 'chromadb-default-embed';
but doesn't have generator it says.

@jeffchuber
Copy link
Contributor Author

jeffchuber commented Jan 10, 2024

@Cinemacloud we havent released the JS client fully yet (though probably will tonight)

can you try installing [1.7.3-beta5](https://www.npmjs.com/package/chromadb/v/1.7.3-beta5) manually and trying that out?

@Cinemacloud
Copy link

Cinemacloud commented Jan 10, 2024

It works thanks!

thorwhalen pushed a commit to thorwhalen/chroma that referenced this pull request Jan 11, 2024
End dx: `npm install chromadb chromadb-default-embed`

`chromadb-default-embed` is a fork of `@xenova/transfomers` to maintain
stability

***

Motivation
- good defaults are good DX
- good defaults lower the barrier to getting started
- currently JS usage is gated on having an API key (seems bad) 

`npm install --save chromadb @xenova/transformers` 

- We want to use the same EF as `python` - `all-MiniLM-L6-v2`
- We want to keep our default package size small (currently 4.5mb)
- We want a happy path for devs getting started - they shouldnt need to
create any accounts or get API keys
- `@xenova/transformers` is great, but it's huge - ~250MB! - so we can't
by default bundle it
- To have a happy path, but keep the bundle size small - we just ask
users to run `npm install --save chromadb @xenova/transformers` to
install chroma. we can add a comment like `// optional default embedding
function`

I also evaluated `https://github.com/visheratin/web-ai` - which is small
(~8MB), but I dont think it supports this model yet? (thought
potentially possible) and
https://github.com/microsoft/onnxruntime/tree/main, which is also
massive (over 100MB).

I confirmed that if you just install `chromadb` and pass
`OpenAIEmbeddingFunction` (or other) - it doesn't complain or yell at
you that you have a missing dep. If you true to use the default and
don't have `@xenova/transformers` installed, it will tell you to use it.

Todo
- [ ] test no require warnings in `nextjs` - this has been an issue in
the past

Thoughts about this DX?
@jeffchuber
Copy link
Contributor Author

@Cinemacloud super

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

Successfully merging this pull request may close these issues.

None yet

5 participants