dottxt-ai / outlines

Structured Text Generation
https://dottxt-ai.github.io/outlines/
Apache License 2.0
9.85k stars 503 forks source link

Samplers don't implement Sampler Protocol #774

Open pmbaumgartner opened 8 months ago

pmbaumgartner commented 8 months ago

What behavior of the library made you think about the improvement?

I was browsing through the source code for Samplers and notice that you declare a Sampler Protocol, but none of the sampler classes inherit from that Protocol.

I think whatever you are using for type checking should be throwing some errors whenever a function takes a sampler and you provide multinomial() as a default arg. For example, I see the following error when trying to declare multinomial() as the default with the type hint of Sampler:

Expression of type "multinomial" cannot be assigned to parameter of type "Sampler"
  "multinomial" is incompatible with protocol "Sampler"
    "__call__" is an incompatible type
      Type "(next_token_logits: DoubleTensor, sequence_weights: DoubleTensor, rng: Generator) -> Tuple[DoubleTensor, DoubleTensor, DoubleTensor]" cannot be assigned to type "(next_token_logits: DoubleTensor, sequence_weights: DoubleTensor, rng: Generator) -> DoubleTensor"
        Function return type "Tuple[DoubleTensor, DoubleTensor, DoubleTensor]" is incompatible with type "DoubleTensor"
          "Tuple[DoubleTensor, DoubleTensor, DoubleTensor]" is incompatible with "DoubleTensor"

How would you like it to behave?

Either the Protocol needs to be updated and then inherited, or the signature of these samplers needs to match the Protocol. Right now the Protocol isn't being used at all.

rlouf commented 7 months ago

Thank you for pointing this out! What do you use for type checking?