-
Notifications
You must be signed in to change notification settings - Fork 484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding MLXLM
, VLLM
classes to LogitsGenerator
type
#970
Conversation
It makes sense that we have an abstract base class for all models, I agree.
Could you help me understand the type hinting error? Does your downstream library have a function / class which accepts any outlines model with the MLXLM isn't a logits generator, it isn't designed to provide logits via |
Ahh ok, I might be misinterpreting that type then. From browsing the code, it seemed to me as though being a For my specific usecase, I was doing something like this (abbreviated semi-pseudo code). Currently I just use def predict(model: LogitsGenerator, gen_type: Literal['generate', 'regex']):
if gen_type == "generate":
generator = outlines.generate.choice(model, ...)
elif gen_type == "regex":
generator = outlines.generate.regex(model, ...)
return generator(prompt) So essentially, I guess my main question is - do you have a type defined within the outlines codebase currently that dictates it will be a valid first argument to a generate method? Or, would this only happening after implementing the abstract base model class you referred to? (Side question - isn't the |
Nope, but I'd like them to. We should have them as an
Based on the name, that seems like a sane definition for |
Very excited about the mlx integration in v0.0.44!
Began integrating it into a project, but noticed that type hinting was throwing some errors around the
LogitsGenerator
type. Essentially, it looks as though this type wasn't synced up with the creation of the two new models.This PR just adds the two models to this type in
models/__init__.py
:However, it brought up a bigger design question I'm curious about. What was the reasoning to go with this pattern (building a custom type using
Union
) as opposed to an abstractLogitsGenerator
class, which all future model classes subclass? This way:LogitsGenerator
generate()
function returning afloat
is a no-go)llamacpp
,mamba
,mlxlm
, etc.) to explicitly return aLogitsGenerator
Thanks!