outlines icon indicating copy to clipboard operation
outlines copied to clipboard

Samplers don't implement Sampler Protocol

Open pmbaumgartner opened this issue 3 months ago • 1 comments

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.

pmbaumgartner avatar Mar 28 '24 01:03 pmbaumgartner