ewfian / faiss-node

Node.js bindings for faiss
https://www.npmjs.com/package/faiss-node
MIT License
107 stars 10 forks source link

Contribution for possible langchainjs integration #3

Closed kyr0 closed 10 months ago

kyr0 commented 1 year ago

Hi,

great project! I'm looking for implementing the FAISS support for langchainjs. I've a bit of a background in writing node native binding modules for Node too and I believe it could be fun focusing on the following subset of features for bridging a currently existing gap and enable langchainjs to support FAISS as well.

Reading the original langchain python code and how FAISS is used there: https://github.com/hwchase17/langchain/blob/master/langchain/vectorstores/faiss.py

... I identified the following APIs need to be supported by the binding:

faiss.IndexFlatL2(length: number): IndexFlatL2
faiss.read_index(name: string): IndexFlatL2
faiss.write_index(index: IndexFlatL2, path: string): void

// on IndexFlatL2:
index.add(embeddings: Float32Array): void
index.search(embedding: Float32Array, k: number): { scores: Float32Array, indices: Int32Array }
index.reconstruct(i: number): Float32Array
index.merge_from(targetIndex: IndexFlatL2): void

A quick read of the code shows that reconstruct and merge_from might be missing in the current implementation: https://github.com/ewfian/faiss-node/blob/main/src/faiss.cc#L51

Is it fine for you if I "mess with the code" and PR? :) Is there anything I should know regarding missing/broken/"I want to refactor this" things that I shouldn't touch? :)

Once I got it working, I can implement and contribute FAISS support for langchainjs here: https://github.com/hwchase17/langchainjs/tree/main/langchain/src/vectorstores

...which I find is a pretty cool goal :)

Thanks and best, Aron

(and sorry, I couldn't change the label to "enhancement" :))

kyr0 commented 1 year ago

Nevermind, I found your PR :)) sry, should have looked in the PRs before. Is there anything else to do?

ewfian commented 1 year ago

@kyr0 Thank you for your detailed proposal. We can consider implementing the reconstruct and merge_from for a complete equivalent to Python's implementation. But the build that was just approved last night failed and we have to get it to pass first.

In the next few days, I will sort out the things to do next. And we can discuss it later.

kyr0 commented 1 year ago

Yeah, cool 👍 Nice to see that the PR looks good now on the other side. Can't wait to use FAISS in my project and port all my code from Python over to TS :)

ewfian commented 1 year ago

@kyr0 Sorry for the late reply. the PR has been merged in langchianjs.

I created a board as a backlog. If you have any good ideas, you can also create issues, and I will add it to the board.

minicacristiancolleva commented 11 months ago

Please close this issue