ewfian / faiss-node

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

Buffers, IP, Factories & more #40

Closed asilvas closed 11 months ago

asilvas commented 1 year ago

Resolves https://github.com/ewfian/faiss-node/issues/37 and much more:

Sorry for the big sweeping changes. I don't like to put this much into 1 PR, but due to considerable complications with NAPI they were necessary.

ewfian commented 1 year ago

@asilvas Thanks a lot for this great PR, please allow me some time to review.

asilvas commented 12 months ago

Let me know if there's anything I can do to progress this PR. Thanks

ewfian commented 12 months ago

@asilvas Sorry for slow reply. I have been occupied with work recently. I'll deal with it later this week.

ewfian commented 11 months ago

Another point that I'm a little concerned about is that Index is an abstract class in faiss. It has only a few methods, but we warped many things that are not originally on Index. maybe we should have a IndexFlat class and a pure Index class?

btw, I'm sorry that it's time for me to go to bed. i will reply tomorrow.

asilvas commented 11 months ago

Another point that I'm a little concerned about is that Index is an abstract class in faiss. It has only a few methods, but we warped many things that are not originally on Index. maybe we should have a IndexFlat class and a pure Index class?

btw, I'm sorry that it's time for me to go to bed. i will reply tomorrow.

Index is abstract, and any index you've created that does not implement a function (ala add_with_ids) will throw. The behavior is highly consistent with the native implementation. All faiss-node needs to be is a wrapper -- let native lib throw when used incorrectly.

ewfian commented 11 months ago

Index is abstract, and any index you've created that does not implement a function (ala add_with_ids) will throw. The behavior is highly consistent with the native implementation. All faiss-node needs to be is a wrapper -- let native lib throw when used incorrectly.

I reconfirmed and it's as you said. But I will remove fromFactory from IndexFlatIP and IndexFlatL2