Skip to content

Conversation

@xenova
Copy link
Contributor

@xenova xenova commented Jun 18, 2023

Currently working on improving how Transformers.js models are viewed and searched for on the hub. I tried to follow this PR as closely as possible. Here's the initial draft, and I plan to make the following changes:

TODO:

  • Add --push_to_hub option to conversion script. Once this is one, I can add the checkbox to /docs/hub/models-libraries.md.
  • Right now, the "Use in X" example code blocks simply use the pipeline_tag and model_id. However, the JS library does still support the traditional AutoModel, AutoTokenizer, AutoProcessor, etc. classes. I plan to add code snippets (/usage example) to each Transformers.js model card on the hub, so, it probably is okay to just keep the example code block as it is.
  • Make sure that the "Use in X" code snippets work with JavaScript code (not only Python).
  • I only list the 17 tasks which are currently supported by the library, but this list will soon grow (especially once WebGPU support is added). I will make follow-up PRs for these.
  • Is there a way to detect whether the model uses a processor, tokenizer, or combination using only the config data? I've seen other examples (like for transformers), but this seems to exclusively choose one or the other.

On another note, I noticed that many tasks supported by transformers (the python library) are not listed in TASKS_MODEL_LIBRARIES (tasks/src/const.ts).

@xenova xenova requested a review from osanseviero June 18, 2023 16:49
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 18, 2023

The documentation is not available anymore as the PR was closed or merged.

@xenova
Copy link
Contributor Author

xenova commented Jun 18, 2023

After checking the GH actions logs to see why it's failing, it looks like it's an auth issue. This is most likely due to the fact that the PR is of a fork instead of a branch. The reason it's a fork is because I originally forked it before I joined HF 😅. I could switch to a branch, but regardless, it should probably still work for forks so that non-HF-members can still contribute. (cc @mishig25 since I know he was busy with similar things)

Edit: Looks like it's now magically working? 👀

Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Thanks a lot! I don't think the TODOs should block this PR, I think we can move ahead with it

@osanseviero osanseviero requested a review from pcuenca June 19, 2023 18:02
Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Excellent! I agree with Omar, we can move forward with this and iterate with the items in the to-do list :)

Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

Add --push_to_hub option to conversion script. Once this is one, I can add the checkbox to /docs/hub/models-libraries.md.

This is quite cool, do you use huggingface.js as a dependency to do that?


To install via [NPM](https://www.npmjs.com/package/@xenova/transformers), run:
```bash
npm i @xenova/transformers
Copy link
Member

Choose a reason for hiding this comment

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

off topic from this PR, but maybe at some point we should ship it under npm i @huggingface/transformers (can be decoupled from moving the actual repo, i.e. i think the repo under your namespace is great)

Copy link
Contributor Author

@xenova xenova Jun 20, 2023

Choose a reason for hiding this comment

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

That would be HUGE! 🚀 We can maybe align the move with a major version release too 👍 (e.g., when WebGPU is supported?). On that note, however, the npm docs say that it isn't possible to transfer a scoped package to another organization. So, we'll either need to create a new package in the new scope, or get custom support for this.

Copy link
Member

Choose a reason for hiding this comment

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

creating a new package is probably fine.

Pinging @coyotte508 to add you to our npm org for when we want to make the switch

Copy link
Member

Choose a reason for hiding this comment

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

@julien-c I think you will have to do it, I don't see the option
image

Alternatively we can add the secret to the repo (like when publishing hf.js), to be able to publish through github actions

xenova and others added 2 commits June 20, 2023 13:09
@xenova
Copy link
Contributor Author

xenova commented Jun 20, 2023

Add --push_to_hub option to conversion script. Once this is one, I can add the checkbox to /docs/hub/models-libraries.md.

This is quite cool, do you use huggingface.js as a dependency to do that?

The conversion script is in python, so I'll just use huggingface_hub for that. However, as mentioned before, the conversion script is just a temporary solution until optimum can perform conversion and quantization in a single step.

@julien-c
Copy link
Member

julien-c commented Jun 20, 2023

Ok i see!
IMO it's not a big deal if the conversion script stays in transformers.js rather than Optimum.
Might even be better actually (keeps code local, etc)

@xenova
Copy link
Contributor Author

xenova commented Jun 20, 2023

Okay I've made the suggested changes (thanks!) 👍

Just one thing I haven't been able to test:

Make sure that the "Use in X" code snippets work with JavaScript code (not only Python).

Is the language (for code highlighting) auto-detected (or is it hard-coded to python)? I don't quite know how to test this.

@julien-c
Copy link
Member

Is the language (for code highlighting) auto-detected (or is it hard-coded to python)?

i don't remember, but we'll see in prod I guess :)

(and if it doesn't work feel free to open an issue in moon-landing)

Copy link
Member

@pcuenca pcuenca left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Very nice! Let's goo 🔥 🚀

@osanseviero osanseviero changed the title [WIP] Add Transformers.js as a supported library Add Transformers.js as a supported library Jun 20, 2023
@osanseviero
Copy link
Contributor

For TODOs, feel free to create separate issues :) I'll merge this

@osanseviero osanseviero merged commit 59f6adb into huggingface:main Jun 20, 2023
@xenova
Copy link
Contributor Author

xenova commented Jun 20, 2023

i don't remember, but we'll see in prod I guess :)

Yep - looks like it defaults to python. e.g.. Not a big deal though, since the copy-pasting still works fine.

image

@pcuenca
Copy link
Member

pcuenca commented Jun 20, 2023

@xenova As Julien said, maybe we can open a (non-urgent) issue in moon-landing :)

@xenova
Copy link
Contributor Author

xenova commented Jun 20, 2023

@pcuenca Done!

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.

6 participants