✨ Better typing for LIBRARY_TASK_MAPPING_EXCLUDING_TRANSFORMERS #552
Conversation
js/src/lib/interfaces/Types.ts
Outdated
| // "doctr": [ | ||
| // "object-detection", | ||
| // ], |
There was a problem hiding this comment.
Commented because it's absent from ModelLibrary
There was a problem hiding this comment.
Same for superb & k2_sherpa
There was a problem hiding this comment.
Should it be handled from the source huggingface/api-inference-community? (see huggingface/api-inference-community#158)
There was a problem hiding this comment.
@osanseviero Any thoughts how to handle the discrepancy. Is the inference API working for doctr, superb & k2_sherpa? Or do we need to add these to ModelLibrary?
I didn't find anything related to k2_sherpa in the hub. I had a quick glimpse on a few superb models and they seem to be using transformer for inference API (?) Most doctr repos are dummy models and the inference API is unable to decide the pipeline_tag.
There was a problem hiding this comment.
Is the inference API working for doctr, superb & k2_sherpa? Or do we need to add these to ModelLibrary?
- Superb repos use the Community Inference API, but given they are not a library and their usage is more for a competition, they were not added to the
Libraries.ts(confirmed with @lewtun). From a discussion, we think https://huggingface.co/models?library=superb&sort=modified are mostly old legacy models and it's ok to remove them from the Inference API, as the official ones now usetransformers. (the old integration allowed users to specify their own requirements and inference code but that was deprecated) - I don't have context in the
k2-sherpaintegration, so maybe makes sense to remove entirely from the community API or exclude from the exporter - https://huggingface.co/models?library=doctr&sort=modified is just one model so I'm fine removing
doctr
There was a problem hiding this comment.
for me this PR is ready to merge
There was a problem hiding this comment.
Thanks! I think adding the two definitely makes sense.
Another issue is that people have been using k2 in HuggingFace tags and k2_sherpa in api-inference-community. I think the easiest fix is to rename k2_sherpa to k2. I'll propose a PR in api-inference-community!
There was a problem hiding this comment.
There was a problem hiding this comment.
and yes renaming k2_sherpa to k2 sounds perfect
There was a problem hiding this comment.
64e8088 was exactly what I had in mind when we discussed @xianbaoqian. :) Thanks for doing it @julien-c!
js/src/lib/interfaces/Types.ts
Outdated
| @@ -1,3 +1,5 @@ | |||
| import type { ModelLibrary } from "./Libraries"; | |||
There was a problem hiding this comment.
i would maybe put LIBRARY_TASK_MAPPING_EXCLUDING_TRANSFORMERS into a new file to prevent the circular imports between Libraries.ts and Types.ts (even if it's just types)
There was a problem hiding this comment.
like for the already existing TASKS_MODEL_LIBRARIES which is very similar, BTW (cc @xianbaoqian)
There was a problem hiding this comment.
(we can change that in the future though – no need to do it in this PR)
|
I pushed a commit to alias the (verbose) type for "model library key" which was also maybe confusing for people Another option was to rename the enum and call the key I have no strong preference between all of these so i went with the least amount of renames, cc @osanseviero |
|
Thanks for making it strong typed! Left a comment. |
| "asteroid": [ | ||
| "audio-source-separation", | ||
| "audio-to-audio" | ||
| // "audio-source-separation", |
osanseviero
left a comment
osanseviero
left a comment
There was a problem hiding this comment.
Nice, overall looks good!
|
Thanks for handling this @julien-c 🤗 |
Co-authored-by: Julien Chaumond <[email protected]>
7 participants
Following #549
Better / stricter typing, allowing only valid ModelLibrary tags and valid PipelineType