xenova / transformers.js

State-of-the-art Machine Learning for the web. Run 🤗 Transformers directly in your browser, with no need for a server!
https://huggingface.co/docs/transformers.js
Apache License 2.0
9.87k stars 582 forks source link

[demo] Add binary & scalar embedding quantization #683

Open jonathanpv opened 2 months ago

jonathanpv commented 2 months ago

This PR addressed the below issue, I've also added an example project showcasing the binary embeddings and using hamming distance function as @xenova gave an example of earlier in the below discussion.

For some reason though I don't get the same similarity numbers as Xenova did on his example but the numbers are at least similar

Hopefully the UI can help debug some of my code if there needs to be changes.

Here's an image of it:

image
xenova commented 2 months ago

Thanks for the PR! Would you mind separating out the example app from the core quantization logic? We can possibly integrate the example app at a later stage, but I think a good starting point for now is to just provide a bit of sample code.

jonathanpv commented 2 months ago

Thanks for the PR! Would you mind separating out the example app from the core quantization logic? We can possibly integrate the example app at a later stage, but I think a good starting point for now is to just provide a bit of sample code.

Ah, yep forgot to this a while back, here it is! Let me know of any changes you'd like

xenova commented 2 months ago

Feel free to rebase off main (which includes your other PR: https://github.com/xenova/transformers.js/issues/681) 🤗

do-me commented 2 months ago

This is awesome! Maybe a little more explanation in the docs would be good. Like what's the (performance) difference between binary and ubinary here and what's the default right now (I guess none)? 

@xenova I'm a little confused that there is no option normal / none. Or is the idea that if you do not pass the parameter, none of the two options is applied and instead it's simply normal (float) precision?

jonathanpv commented 2 months ago

This is awesome! Maybe a little more explanation in the docs would be good. Like what's the (performance) difference between binary and ubinary here and what's the default right now (I guess none)? 

@xenova I'm a little confused that there is no option normal / none. Or is the idea that if you do not pass the parameter, none of the two options is applied and instead it's simply normal (float) precision?

yep idea is to not pass the param if you don't plan on using it, its set as a default param / named param

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters

I can improve the docs for this for sure

do-me commented 2 months ago

Thanks for your reply. Found the related line here:

    if (!['binary', 'ubinary'].includes(precision)) {
        throw new Error("The precision must be either 'binary' or 'ubinary'");
    }

I understand the idea of the paradigm. However, it causes silightly more work for downstream applications. In my case, I wanted to create a dropdown menu for UI users with 3 values:

I would always pass the precision param as it's the easiest to do in client-side JS. However, due to the current logic I need to add more complexity, checking whether a user selected normal mode and not pass the precision param in the function call in case. Is there any advantage in not including the normal mode or would you mind adding it?

jonathanpv commented 2 months ago

Thanks for your reply. Found the related line here:

    if (!['binary', 'ubinary'].includes(precision)) {
        throw new Error("The precision must be either 'binary' or 'ubinary'");
    }

I understand the idea of the paradigm. However, it causes silightly more work for downstream applications. In my case, I wanted to create a dropdown menu for UI users with 3 values:

  • binary
  • ubinary
  • normal/full/float (however you want to call it)

I would always pass the precision param as it's the easiest to do in client-side JS. However, due to the current logic I need to add more complexity, checking whether a user selected normal mode and not pass the precision param in the function call in case. Is there any advantage in not including the normal mode or would you mind adding it?

Hmm good call out, the reason I added quantize was to make it explicit you'd be quantizing. It technically does add more complexity from a UI / code perspective

Trying to think if quantize yes/no would add another field besides precision, if so then quantizeOptions would need to be created, can't think of anything at the moment.

technically we could flatten out the param to just be 'precision'

to be super clear in your suggestion:

your current scenario makes your UI render the dropdown conditionally based on if the 'quantize embedding' is set to true eg

checkbox:
checkbox label: quantize embedding?

dropdown:
if checkbox yes, then render precision options

vs just having precision his ui would be the three dropdown

precision dropdown:
ubinary
binary
float32 (full)

So basically right now the cases are:

cases now: 4


quantize: false
precision: binary (this is ignored)

===

quantize: false
precision: ubinary (this is ignored)

===

quantize: true
precision: binary

===

quantize: true
precision: ubinary

cases if flattend: 3

precision: binary
===
precision: ubinary
===
precision: float32 / full ?
do-me commented 2 months ago

UI

your current scenario makes your UI render the dropdown conditionally based on if the 'quantize embedding' is set to true eg

Exactly. One dropdown with all 3 flattened options would be much easier to use than conditionally changing the UI and JSON I need to pass to the function.

--

By the way: there are now two quantize/d options in transformers.js, one for loading a quantized model in pipeline and one for your precision settings.

Th3G33k commented 2 months ago

+1 with do-me